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.

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

Fix ec2_snapshot_facts for python3 #30606

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

willthames
Copy link
Contributor

SUMMARY

Avoid the following seen when running ec2_ami tests on python3,
presumably because the return type of map is different between
python2 and python3.

Traceback (most recent call last):
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 242, in <module>
    main()
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 238, in main
    list_ec2_snapshots(connection, module)
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 193, in list_ec2_snapshots
    snapshots = connection.describe_snapshots(SnapshotIds=snapshot_ids, OwnerIds=owner_ids, RestorableByUserIds=restorable_by_user_ids, Filters=filters)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 312, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 575, in _make_api_call
    api_params, operation_model, context=request_context)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 630, in _convert_to_request_dict
    api_params, operation_model)
  File "/usr/local/lib/python3.5/dist-packages/botocore/validate.py", line 291, in serialize_to_request
    raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter OwnerIds, value: <map object at 0x7ff577511048>, type: <class 'map'>, valid types: <class 'list'>, <class 'tuple'>

#30435 (comment)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_snapshot_facts

ANSIBLE VERSION
ansible 2.5.0 (devel 37736ee87e) last updated 2017/09/20 16:37:19 (GMT +1000)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/will/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/will/src/ansible/lib/ansible
  executable location = /home/will/src/ansible/bin/ansible
  python version = 2.7.13 (default, Sep  5 2017, 08:53:59) [GCC 7.1.1 20170622 (Red Hat 7.1.1-3)]

@ansibot
Copy link
Contributor

ansibot commented Sep 20, 2017

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 aws bugfix_pull_request cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. python3 support:community This issue/PR relates to code supported by the Ansible community. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 20, 2017
@@ -185,7 +185,7 @@
def list_ec2_snapshots(connection, module):

snapshot_ids = module.params.get("snapshot_ids")
owner_ids = map(str, module.params.get("owner_ids"))
owner_ids = module.params.get("owner_ids")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops running str on the owner_ids. This could have an impact if the API these are used with expect strings but an int is passed in, for instance. If the str is still needed, you can do something like:

owner_ids = [str(i) for i in module.params.get("owner_ids")]

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Sep 21, 2017
@abadger
Copy link
Contributor

abadger commented Sep 21, 2017

@willthames I left one comment but I don't know the API being used so I don't know if it's needed or not. Once you know the answer to that (either it's fine the way it is or modify the code to handle that corner case), feel free to merge this in. You can also cherry-pick to stable-2.4 (or request that I do so on irc).

Avoid the following seen when running ec2_ami tests on python3,
presumably because the return type of `map` is different between
python2 and python3.

```
Traceback (most recent call last):
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 242, in <module>
    main()
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 238, in main
    list_ec2_snapshots(connection, module)
  File "/tmp/ansible_e44v27uj/ansible_module_ec2_snapshot_facts.py", line 193, in list_ec2_snapshots
    snapshots = connection.describe_snapshots(SnapshotIds=snapshot_ids, OwnerIds=owner_ids, RestorableByUserIds=restorable_by_user_ids, Filters=filters)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 312, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 575, in _make_api_call
    api_params, operation_model, context=request_context)
  File "/usr/local/lib/python3.5/dist-packages/botocore/client.py", line 630, in _convert_to_request_dict
    api_params, operation_model)
  File "/usr/local/lib/python3.5/dist-packages/botocore/validate.py", line 291, in serialize_to_request
    raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter OwnerIds, value: <map object at 0x7ff577511048>, type: <class 'map'>, valid types: <class 'list'>, <class 'tuple'>
```

ansible#30435 (comment)
@willthames
Copy link
Contributor Author

@abadger, a great spot, thank you - I tested this and you were quite correct. I've updated the fix and applied a similar fix to restorable_by_user_ids

@abadger abadger merged commit 5900fee into ansible:devel Sep 28, 2017
@abadger
Copy link
Contributor

abadger commented Sep 28, 2017

Merged to devel and cherry-picked for the 2.4.1 release.

@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 aws bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. python3 support:community This issue/PR relates to code supported by the Ansible community.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants