-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
create a version that supports boto3/python3, updated requirements file #26
base: master
Are you sure you want to change the base?
Conversation
Thanks for this. I am a little bit hesitant to merge it as is because of the significant amount of code duplication, it might create a maintainability nightmare moving forward, requiring to cherry pick each change into both files. Can you by any chance think of a way to implement the support to Python 2 + 3 that is more idiomatic, perhaps in the same script? This might not be possible (admittedly I've never used boto3, so I'm not aware of how much changed), so I'm just asking for my information. Thanks again |
well boto3 is supported under python2 so I could write in python2 that is compatible with python3. |
@gianlucaborello this is now pep8 compliant and works in both python2 / python3 using boto3 |
@gianlucaborello any chance of merging? |
Handle no tags
Sorry, I've been super busy and had no time to review this. I will be able to take a look and test it after things calm down a little from my end. Thanks. |
instance_id = value | ||
else: | ||
instance_id += '-' + value | ||
for aws_tag in instance['Instances'][0].get('Tags', []): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a minor copy/paste error. As it is currently if you pass --tags Name
, it will iterate through all instance tags and append with -, regardless of what tags were passed.
I added the following break here, and now works as expected
if aws_tag['Key'] != tag:
continue
Sent a PR GremlinLTD#2 Not a python guy, also unfamiliar with boto3 lib, so there's probably a better way without this branching but i'm just limiting my exposure so i dont end up in the weeds. I'm now using this in my ansible playbooks. Here's my usage in production. It's working well with the updated version, and i removed my python2 dependency. - name: Update ssh config
shell: ./aws-ssh-config.py --private --tags Name --default-user ubuntu > .ssh/config
become_user: ubuntu
args:
chdir: /home/ubuntu |
Skip appending a tag value if it's not in the given filter
looks good, I wonder if you would be able to generate the same thing using the Ansible dynamic inventories :-) |
@gianlucaborello any luck getting this in |
@gianlucaborello appreciate that you've probably had other things going on, if you would like I am happy to become a collaborator on this with you to keep development up |
Tag order is non-deterministic, so instances with the same tags set might end up not getting sequential names, e.g. 'foo-bar' and 'bar-foo' instead of 'foo-bar-1' and 'foo-bar-2'.
Sort tags by key
add newer versions of python
Add newer versions of python for automated build testing
Hi @gianlucaborello, Hope all is well, any chance we can look to get these merged? I would be happy to take over this repo if you no-longer have the time Trozz |
Just a little nudge to show interest in this merge. |
Wanted to bump this as well, would be great to have an update |
added a version of the file support Python 3 and boto3, functionality is the same still
if you want I will rewrite the base file as well as submit a setup.py file to support running and installing the script as an application