Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix python3 get imageid #52936

Merged
merged 6 commits into from
Jun 26, 2019
Merged

Fix python3 get imageid #52936

merged 6 commits into from
Jun 26, 2019

Conversation

NiCalderon
Copy link

What does this PR do?

Expose, test and fix a python 3 salt-cloud bug when using an image not starting with "ami-"

What issues does this PR fix or reference?

None

Previous Behavior

   Traceback (most recent call last):
     File "/usr/lib/python3/dist-packages/mock/mock.py", line 1305, in patched
       return func(*args, **keywargs)
     File "/home/nicolasc/salt/tests/unit/cloud/clouds/test_ec2.py", line 104, in test__get_imageid_by_name
       imageid = ec2._get_imageid_from_image_name('CentOS Linux 7*')
     File "/home/nicolasc/salt/salt/cloud/clouds/ec2.py", line 1237, in _get_imageid_from_image_name
       lambda i, j: salt.utils.compat.cmp(_t(i), _t(j))
   TypeError: sorted expected 1 arguments, got 2

New Behavior

It works™

Tests written?

Yes

Commits signed with GPG?

Yes

PS: I almost forgot to pick the right branch to merge. I feel like it's easy to overlook.

NiCalderon added 3 commits May 7, 2019 20:58
@NiCalderon
Factor out ami-* imageId lookup from get_imageId
69cd4e6 
Separation of concerns, and reduces the amount of mocking that needs to be
done in order to isolate bug.
@NiCalderon
Add test case for salt-cloud ec2's get_imageid
332dcd3 
The mock values were obtained by running an acutal query, truncating
the list of returned entries and stripping unnecessary keys from the
results.

The test case runs well with python 2 but causes a traceback with
python 3.
@NiCalderon
Fix salt-cloud traceback with python 3
99831c7 
With python 3, salt-cloud tracebacks in salt/cloud/clouds/ec2.py's
get_imageid when searching for an image name not starting with "ami-".
Python 3 removed the cmp parameter in favor of key.
@NiCalderon NiCalderon mentioned this pull request May 8, 2019
Closed
@NiCalderon
Copy link
Author

The failed test case is unrelated, and I'm told it failed on other unrelated PR. Is there anything preventing this from being merged in?

@Ch3LL Ch3LL requested a review from xeacott June 25, 2019 20:51
@xeacott xeacott merged commit 1068893 into saltstack:2019.2 Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmcmarrow cmcmarrow cmcmarrow approved these changes

@xeacott xeacott xeacott approved these changes

Assignees
No one assigned
Labels
code-jam Reviewers-Assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@NiCalderon @xeacott @cmcmarrow @Ch3LL @sagetherage @twangboy