ENH: Added StarCluster sshutils so users can log into AWS EC2 instances.#101
ENH: Added StarCluster sshutils so users can log into AWS EC2 instances.#101yarikoptic merged 14 commits intoReproNim:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #101 +/- ##
==========================================
- Coverage 80.44% 71.88% -8.56%
==========================================
Files 108 114 +6
Lines 6167 7499 +1332
==========================================
+ Hits 4961 5391 +430
- Misses 1206 2108 +902
Continue to review full report at Codecov.
|
| else: | ||
| raise NotImplementedError("Bad --backend paramenter '{}'".format(key)) | ||
| if backend: | ||
| config = backend_set_config(backend, env_resource, config) |
There was a problem hiding this comment.
imho better to have a function without 'side effect' of modifying config in place and then also returning it... what if
backend_set_config -> get_backend_config(backend, env_resource) which returns a backend_config, and then you just do config.update(backend_config), i.e. update with those new settings. IMHO makes it clearer
|
|
||
| # Set resource properties to any backend specific command line arguments. | ||
| if backend: | ||
| config = backend_set_config(backend, env_resource, config) |
There was a problem hiding this comment.
same applies as above... I feel that it all might become also shorter after we look into making ResourceManager a proper class etc, but that later
| lgr.debug("Opening TTY connection to AWS EC2 instance.") | ||
| host = self._ec2_instance.public_ip_address | ||
| ssh = SSHClient(host, private_key=self.key_filename) | ||
| ssh.interactive_shell(self.user) |
There was a problem hiding this comment.
could we also get a simple "SSHHost" resource or smth like that so we could register/login to remote hosts (which do not need startup etc) general?
| if line != '': | ||
| output.append(line) | ||
| print line, | ||
| print(line), |
There was a problem hiding this comment.
oh -- starcluster is not python3 compatible? we are asking for trouble I am afraid... we might want to just futurize its source code or smth like that at least...
|
Ok @mjtravers -- I will merge it so I could merge these changes with my branch as well since tests pass already, and then we could address those comments I have left. |
3 participants
Pull request for SSHing into AWS EC2 instance using StarCluster utils.