Boto3 conversion

[pmkane]

Hi there:

This a new PR to convert aws-ec2-assign-elastic-ip to use boto3, instead of boto.

Because boto3 still doesn't have get_instance_metadata() equivalent, it adds ec2-metadata as a requirement.

The only functional change in this PR is that AllowReassociation is always False, whereas in the current boto implementation, whether it is True or False depends on the date that your EC2 environment was created. See https://github.com/skymill/aws-ec2-assign-elastic-ip/issues/24 for more details.

I've tested the use cases that broke in https://github.com/skymill/aws-ec2-assign-elastic-ip/issues/28 or https://github.com/skymill/aws-ec2-assign-elastic-ip/issues/29 and they both work correctly in this branch.

[pmkane]

Just a quick bump on this -- if you'd prefer to stick with boto2, given the problems from the last release, I understand, but we will want to continue work in our fork to add features.

[przemyslaw-elias]

@sebdah It appears that we also ran into reassociation bug. Because the default has changed, it's impossible to send valid request with old boto. In order to fix this, a client change to boto3 is required.

Would you be willing to look into this?

[sebdah]

I'm really swamped with dealing with Covid-19 stuff at work, if someone could review the code and give this branch a go I'd be happy to merge, package and release.

[pmkane]

Sure. I’ll do it this weekend.

On Fri, Apr 3, 2020 at 1:59 PM Przemysław Elias notifications@github.com wrote:

@przemyslaw-elias approved this pull request.

I'm not a python expert, but the code looks okay to me :)

Tested this branch on EC2 with the following: pip install ec2_metadata git+ https://github.com/pmkane/aws-ec2-assign-elastic-ip.git@boto3_conversion, worked well.

@pmkane https://github.com/pmkane Could you update to master?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/skymill/aws-ec2-assign-elastic-ip/pull/32#pullrequestreview-387507143, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAL5KV5A7KMI5LQUJWHMJD3RKYWX3ANCNFSM4GYWZT2A .

-- Patrick Michael Kane p@patrick.com

[pmkane]

Done.

[sebdah]

Awesome, thanks guys. I'll get to this a little later today.

[sebdah]

Thanks for the help here @pmkane and @przemyslaw-elias. I have released version 0.10.0 now.

[przemyslaw-elias]

Guys, it looks like the merge to master didn't go as planned. Before that it's been working properly, but now it's giving an error:

Traceback (most recent call last):
  File "/usr/local/bin/aws-ec2-assign-elastic-ip", line 31, in <module>
    aws_ec2_assign_elastic_ip.main()
  File "/usr/local/lib/python2.7/site-packages/aws_ec2_assign_elastic_ip/__init__.py", line 59, in main
    address = _get_unassociated_address()
  File "/usr/local/lib/python2.7/site-packages/aws_ec2_assign_elastic_ip/__init__.py", line 115, in _get_unassociated_address
    all_addresses = connection.get_all_addresses()
  File "/usr/local/lib/python2.7/site-packages/botocore/client.py", line 566, in __getattr__
    self.__class__.__name__, item)

@pmkane in your PR you changed connection.get_all_addresses to connection.describe_addresses, but the same code has slipped through merge to master.

Moreover, I don't know whats wrong, but I had to manually run pip install ec2_metadata because it's been missing.

[sebdah]

Moreover, I don't know whats wrong, but I had to manually run pip install ec2_metadata because it's been missing.

Hmm, yes, there is a new dependency added. It must be added to the list of dependencies. Also it seems like the boto dependency should be bumped.

I'll prepare a bugfix PR shortly.

[pmkane]

Ugh, sorry -- I clearly rushed the merge. I am happy to submit a PR to clean up my own mess. Will do so, unless @sebdah has already gotten there.

[pmkane]

Looks like you did, thank you @sebdah.