Add support for adding custom query parameters to URL #46390

remyleone · 2018-10-02T10:57:31Z

SUMMARY

It is currently not possible to specify query parameters to Scaleway URL. For instance, it is not currently possible to specify how many results you want per page. It makes it difficult to test pagination for Scaleway compute without hitting the default number of machines per pages (50).

By adding arbitrary queries parameters to each ansible modules we will be able to have a greater flexibility in our API calls.

Will be very useful to test #46117

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

scaleway

ANSIBLE VERSION

ansible 2.8.0.dev0 (scaleway_query_filters 2c9caa04d2) last updated 2018/10/02 12:53:33 (GMT +200)
  config file = None
  configured module search path = [u'/Users/sieben/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sieben/workspace/ansible/lib/ansible
  executable location = /Users/sieben/workspace/ansible/bin/ansible
  python version = 2.7.15 (default, Jun 17 2018, 12:46:58) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)]

ansibot · 2018-10-02T10:58:58Z

cc @Spredzy @abarbare @Anthony25 @hekonsek @pilou-
click here for bot help

ansibot · 2018-10-02T11:07:10Z

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/module_utils/scaleway.py:34:17: undefined-variable Undefined variable 'ALLOWED_DEVICE_QUERY_PARAMETERS'
lib/ansible/module_utils/scaleway.py:36:77: undefined-variable Undefined variable 'ALLOWED_DEVICE_QUERY_PARAMETERS'
lib/ansible/module_utils/scaleway.py:45:71: undefined-variable Undefined variable 'urlencode'

click here for bot help

ansibot · 2018-10-12T18:46:37Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module scaleway_compute scaleway_image_facts scaleway_ip scaleway_ip_facts scaleway_organization_facts scaleway_security_group scaleway_security_group_facts scaleway_security_group_rule scaleway_server_facts scaleway_snapshot_facts scaleway_sshkey scaleway_user_data scaleway_volume scaleway_volume_facts" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from /root/ansible/lib/ansible/modules
/cloud/scaleway/scaleway_security_group_facts.py, line 13, column 5, found a
duplicate dict key (choices). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module scaleway_compute scaleway_image_facts scaleway_ip scaleway_ip_facts scaleway_organization_facts scaleway_security_group scaleway_security_group_facts scaleway_security_group_rule scaleway_server_facts scaleway_snapshot_facts scaleway_sshkey scaleway_user_data scaleway_volume scaleway_volume_facts" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from /root/ansible/lib/ansible/modules
/cloud/scaleway/scaleway_security_group_facts.py, line 13, column 5, found a
duplicate dict key (choices). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module scaleway_compute scaleway_image_facts scaleway_ip scaleway_ip_facts scaleway_organization_facts scaleway_security_group scaleway_security_group_facts scaleway_security_group_rule scaleway_server_facts scaleway_snapshot_facts scaleway_sshkey scaleway_user_data scaleway_volume scaleway_volume_facts" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from /root/ansible/lib/ansible/modules
/cloud/scaleway/scaleway_security_group_facts.py, line 13, column 5, found a
duplicate dict key (choices). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module scaleway_compute scaleway_image_facts scaleway_ip scaleway_ip_facts scaleway_organization_facts scaleway_security_group scaleway_security_group_facts scaleway_security_group_rule scaleway_server_facts scaleway_snapshot_facts scaleway_sshkey scaleway_user_data scaleway_volume scaleway_volume_facts" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from /root/ansible/lib/ansible/modules
/cloud/scaleway/scaleway_security_group_facts.py, line 13, column 5, found a
duplicate dict key (choices). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module scaleway_compute scaleway_image_facts scaleway_ip scaleway_ip_facts scaleway_organization_facts scaleway_security_group scaleway_security_group_facts scaleway_security_group_rule scaleway_server_facts scaleway_snapshot_facts scaleway_sshkey scaleway_user_data scaleway_volume scaleway_volume_facts" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from /root/ansible/lib/ansible/modules
/cloud/scaleway/scaleway_security_group_facts.py, line 13, column 5, found a
duplicate dict key (choices). Using last defined value only.

The test ansible-test sanity --test validate-modules [explain] failed with 14 errors:

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_image_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_ip.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_ip_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_organization_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group_rule.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_server_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_snapshot_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_sshkey.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_user_data.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_volume.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_volume_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/cloud/scaleway/scaleway_security_group_facts.py:31:5: key-duplicates DOCUMENTATION: duplication of key "choices" in mapping

click here for bot help

ansibot · 2018-10-12T18:46:37Z

cc @mattclay
click here for bot help

remyleone · 2018-10-15T11:58:24Z

@Spredzy @pilou- could I get a review?

ansibot · 2018-10-15T13:09:00Z

The test ansible-test sanity --test validate-modules [explain] failed with 14 errors:

lib/ansible/modules/cloud/scaleway/scaleway_compute.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_image_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_ip.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_ip_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_organization_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_security_group_rule.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_server_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_snapshot_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_sshkey.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_user_data.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_volume.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/scaleway/scaleway_volume_facts.py:0:0: E322 "query_parameters" is listed in the argument_spec, but not documented in the module

click here for bot help

remyleone · 2018-10-18T08:29:55Z

@gundalow Could you review and merge?

remyleone · 2018-10-19T08:28:02Z

@pilou- Could you review?

mscherer · 2018-10-19T11:50:03Z

So, I am a bit concerned about the level of code duplication between module_utils/scaleway.py and the online.py one, especially since we start to change one to add more flexibility that could end being useful to the others. And also concerned about cut and paste with DO module_utils.

mscherer · 2018-10-19T11:52:30Z

shipit

remyleone · 2018-10-19T11:57:31Z

shipit

pilou- · 2018-10-19T12:05:41Z

I have a negative opinion on this PR:

pagination settings must not be module parameters instead use something like max_results
if a user really would to do that, he could use uri module

remyleone · 2018-10-19T12:14:08Z

@pilou- This PR is not about pagination. This PR is about adding the possibility to add query_parameters to the URL from the module. This can be used to offer the possibility to the user to add optional filters to speed up queries for instance if they want to and if the module does not support it natively.

pilou- · 2018-10-19T12:20:58Z

Users still can use URI module :) I am not convinced by the given examples. And what about the idempotence ? Should not this feature be used only for facts modules ?

remyleone · 2018-10-19T15:53:07Z

I don't understand what idempotence have to do with this PR. I also don't understand why it should also only be used for facts? Could you give examples?

lib/ansible/utils/module_docs_fragments/scaleway.py

remyleone · 2018-10-20T15:39:18Z

@halberom How do you handle the case where you have several time the same key in a url query string? Espically with urlencode?

halberom · 2018-10-20T17:20:18Z

@sieben, not sure what you mean. I made my comment because it looked like you had typos on the doc of the param. It states type: list but has default: {}. If it should be a list (of dicts), then your default is wrong (and possibly your examples and code).

remyleone · 2018-10-26T12:15:09Z

@gundalow Could this be merged?

gundalow · 2018-10-26T14:20:26Z

Merged into devel for release in Ansible 2.8.

Thanks for the PR and reviews!

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Oct 2, 2018

webknjaz removed the needs_triage Needs a first human triage before being processed. label Oct 2, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 10, 2018

remyleone force-pushed the scaleway_query_filters branch from 2c9caa0 to 3da8a1f Compare October 12, 2018 15:52

remyleone force-pushed the scaleway_query_filters branch 2 times, most recently from f151022 to 286f1ca Compare October 15, 2018 11:57

remyleone changed the title ~~WIP: Add support for adding custom query parameters to URL~~ Add support for adding custom query parameters to URL Oct 15, 2018

ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Oct 15, 2018

ansibot added the core_review In order to be merged, this PR must follow the core review workflow. label Oct 15, 2018

ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Oct 19, 2018

halberom reviewed Oct 20, 2018

View reviewed changes

lib/ansible/utils/module_docs_fragments/scaleway.py Show resolved Hide resolved

remyleone force-pushed the scaleway_query_filters branch from 10048b1 to c373184 Compare October 26, 2018 12:11

Add support for adding custom query parameters to URL

407e46b

remyleone force-pushed the scaleway_query_filters branch from c373184 to 407e46b Compare October 26, 2018 12:14

ansibot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Oct 26, 2018

gundalow merged commit 3386058 into ansible:devel Oct 26, 2018

Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this pull request Dec 4, 2018

Add support for adding custom query parameters to URL (ansible#46390)

413d897

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for adding custom query parameters to URL #46390

Add support for adding custom query parameters to URL #46390

remyleone commented Oct 2, 2018 •

edited

ansibot commented Oct 2, 2018

ansibot commented Oct 2, 2018

ansibot commented Oct 12, 2018

ansibot commented Oct 12, 2018

remyleone commented Oct 15, 2018

ansibot commented Oct 15, 2018

remyleone commented Oct 18, 2018

remyleone commented Oct 19, 2018

mscherer commented Oct 19, 2018

mscherer commented Oct 19, 2018

remyleone commented Oct 19, 2018

pilou- commented Oct 19, 2018

remyleone commented Oct 19, 2018

pilou- commented Oct 19, 2018

remyleone commented Oct 19, 2018

remyleone commented Oct 20, 2018

halberom commented Oct 20, 2018

remyleone commented Oct 26, 2018

gundalow commented Oct 26, 2018

Add support for adding custom query parameters to URL #46390

Add support for adding custom query parameters to URL #46390

Conversation

remyleone commented Oct 2, 2018 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Oct 2, 2018

ansibot commented Oct 2, 2018

ansibot commented Oct 12, 2018

ansibot commented Oct 12, 2018

remyleone commented Oct 15, 2018

ansibot commented Oct 15, 2018

remyleone commented Oct 18, 2018

remyleone commented Oct 19, 2018

mscherer commented Oct 19, 2018

mscherer commented Oct 19, 2018

remyleone commented Oct 19, 2018

pilou- commented Oct 19, 2018

remyleone commented Oct 19, 2018

pilou- commented Oct 19, 2018

remyleone commented Oct 19, 2018

remyleone commented Oct 20, 2018

halberom commented Oct 20, 2018

remyleone commented Oct 26, 2018

gundalow commented Oct 26, 2018

remyleone commented Oct 2, 2018 •

edited