Move follow_redirects parameter

[monsdar]

SUMMARY

Moves the follow_redirects parameter to the url_argument_spec function so external modules like community.general.rundeck_project could use it as well without having to copy it.

Fixes #82335

ISSUE TYPE

• Feature Pull Request

[felixfontein]

You also need to update the docs fragment (and move the documentation there).

I'm not sure this is a good solution though, since this is an option that shouldn't be always there for modules using this. It should rather be opt-in, i.e. have another function like url_argument_spec_redirect() in lib/ansible/module_utils/urls.py and another docs fragment (f.ex. ansible.builtin.url.redirect) for the corresponding docs. Then modules can opt-in to this feature if it makes sense for them.

(TBH it probably also makes sense to split up the existing url_argument_spec into different parts. Right now most modules using the module utils only need a subset of it. Having multiple composable parts would make it a lot easier to reuse the parts you actually need. But that's more of a long-term project and needs discussion with the Core team first, I guess.)

[monsdar]

I made the redirect-spec optional by adding it into its own function. I also created a corresponding doc_fragment and made ansible.builtin.uri use this instead of defining the parameter itself.

Offtopic: I'm a bit wondered why the module isn't using the uri-doc_fragment at all. Is this on purpose? Perhaps the docs should be merged into the doc_fragment.

[felixfontein]

You definitely also need a changelog fragment.

Also please note that the url lookup also has follow_redirects, although with a different description, a different default, a version_added, etc. Here you can use the ability to overwrite docs fragments, by including the docs fragment, and specifying

  follow_redirects:
    description: String of urllib2, all/yes, safe, none to determine how redirects are followed
    version_added: "2.10"
    default: 'urllib2'
    vars:
        - name: ansible_lookup_url_follow_redirects
    env:
        - name: ANSIBLE_LOOKUP_URL_FOLLOW_REDIRECTS
    ini:
        - section: url_lookup
          key: follow_redirects

[felixfontein]

I would personally not add a new docs fragment, but add a sub-docs fragment to the url docs fragment. Not sure what the Core team prefers though.

[monsdar]

I cannot find any docs on this, but it sounds like the way to go. Could you give me a pointer on how to implement it?

[felixfontein]

See for example:

• https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/doc_fragments/action_common_attributes.py#L53
• https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/import_playbook.py#L26

[monsdar]

I added the sub-doc fragment. I'm unsure about the uri-lookup docs though. The default there is urllib2 while the uri-module has safe as default. Would this be overwritten in a wrong manner?

[monsdar]

Sorry for the long pause, Christmas got in the way 👀

[ansibot]

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

lib/ansible/plugins/doc_fragments/url.py:89:0: missing-final-newline: Final newline missing

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

lib/ansible/plugins/doc_fragments/url.py:89:4: W292: no newline at end of file

click here for bot help