3 ms·
Nice idea, but needs a bit of refactoring: hardcoded paths like '/tmp/.ssh2_list' should not be used. This is insecure and wouldn't work on (most) multi user s
by justinsaccount 11y ago
Nice idea, but needs a bit of refactoring:
hardcoded paths like '/tmp/.ssh2_list' should not be used. This is insecure and wouldn't work on (most) multi user systems. Something like os.path.expanduser("~/.cache/ssh2_list") should be used instead.
f = open(cache_file_list, 'w')
f.write(output)
f.close()
can be written as
with open(cache_file_list, 'w') as f:
f.write(output)
This:
i = 0
for instance in all_instances:
i += 1
#..
print '%-4s%-55s%-30s' % ('[' + str(i) + ']',
instance['PublicDnsName'], name)
Simplifies to:
for i, instance in enumerate(all_instances, 1):
choice = '[%d]' % i
name = extract_name(instance) #function instead of copy/paste
print '%-4s %-55s %-30s' % (choice, instance['PublicDnsName'], name)
Also would kind of make more sense to support matching on the name of the instance rather than requiring a number. In your case, 'ssh2 cron' makes a bit more sense than 'ssh2 0'
- voltagex_ 11y agoMight be better to open an issue. https://github.com/soheil/ssh2/issues https://github.com/soheil/ssh2/issues I'd do it, but I have to run.
- soheil 11y agoJust made those changes, will update the tmp path.
- jessedhillon 11y agoSince this is Python, you can use the built-in tempfile module to securely create/delete temporary files https://docs.python.org/2/library/tempfile.html https://docs.python.org/2/library/tempfile.html Additionally, a feature where the command line argument is applied as a wildcard search against the list of hosts would be nice, with the default behavior being to ssh into the first match when there's only one matching host.
- drdaeman 11y ago> os.path.expanduser("~/.cache/ssh2_list") Please, consider $XDG_CACHE_HOME. Not everyone likes the default directory layout. I think, ugly-looking but reasonably portable version would look like os.path.join(os.environ.get("XDG_CACHE_HOME", os.path.join(os.path.expanduser("~"), ".cache")), "ssh2_list")
- soheil 11y agoTook care of this, thanks for the tip.