Skip to content

Make sure collection name is included in plugin deprecations from collections - #73239

Closed
felixfontein wants to merge 3 commits into
ansible:develfrom
felixfontein:fix-plugin-deprecations-1
Closed

Make sure collection name is included in plugin deprecations from collections#73239
felixfontein wants to merge 3 commits into
ansible:develfrom
felixfontein:fix-plugin-deprecations-1

Conversation

@felixfontein

@felixfontein felixfontein commented Jan 14, 2021

Copy link
Copy Markdown
Contributor
SUMMARY

Make sure collection name is included in plugin deprecations from collections.

(One part of #73058.)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/plugins/loader.py

@ansibot ansibot added affects_2.11 botmeta bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Jan 14, 2021
@felixfontein
felixfontein force-pushed the fix-plugin-deprecations-1 branch from 3f622bc to 6355511 Compare January 14, 2021 21:13
@felixfontein felixfontein mentioned this pull request Jan 14, 2021
Closed
@felixfontein

felixfontein commented Jan 14, 2021

Copy link
Copy Markdown
Contributor Author

A test for this is contained in #73240.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review labels Jan 14, 2021
@felixfontein

felixfontein commented Jan 15, 2021

Copy link
Copy Markdown
Contributor Author

/rebuild_failed

@ansibot ansibot added core_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 15, 2021
felixfontein
felixfontein commented Jan 17, 2021
View reviewed changes
Comment thread lib/ansible/plugins/loader.py
self._module_cache[path] = self._load_module_source(name, path)
self._load_config_defs(name, self._module_cache[path], path)
self._load_config_defs(
name, self._module_cache[path], path, collection_name=plugin_load_context.plugin_resolved_collection)

@felixfontein felixfontein Jan 17, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bcoca including the collection name in name isn't trivial, since sometimes name is not a short name, but something else; for example I saw ansible_collections.ansible.builtin.plugins.lookup.file when running the following playbook:

- hosts: localhost
  tasks:
    - debug:
        msg: "{{ lookup('ansible.builtin.file', 'asdf') }}"

I guess that this shouldn't happen and should be fixed, but without that fixed I don't see how to combine name + collection name into one parameter without introducing a new syntax (that uses another separator).

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. core_review and removed core_review needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 17, 2021
@Shrews Shrews removed the needs_triage Needs a first human triage before being processed. label Jan 19, 2021
@ansibot 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 Jan 27, 2021
@felixfontein
felixfontein force-pushed the fix-plugin-deprecations-1 branch from b61a863 to f9f1876 Compare November 4, 2021 18:56
@ansibot ansibot removed 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 Nov 4, 2021
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 4, 2021
@ansibot 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 Nov 16, 2021
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review labels May 5, 2022
@mattclay

mattclay commented Mar 22, 2023

Copy link
Copy Markdown
Member

@felixfontein Is this still something you're working on and would like to rebase?

@felixfontein

felixfontein commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@mattclay if there is a chance that this will actually get reviewed and merged, I would like to rebase and update it.

@mattclay

mattclay commented Mar 22, 2023

Copy link
Copy Markdown
Member

@felixfontein Is there benefit to merging this without also having #73240?

@felixfontein

felixfontein commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

It doesn't really need #73240, but without a fix for #73051 this is probably only helpful in limited cases (maybe inventory or vars plugins - I'd have to test).

@mattclay

mattclay commented Mar 22, 2023

Copy link
Copy Markdown
Member

OK, perhaps we're better off waiting until #73051 is resolved then.

@briantist briantist mentioned this pull request Jun 17, 2023
Merged
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 12, 2023
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Oct 24, 2023
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@mattclay

mattclay commented Jun 5, 2025

Copy link
Copy Markdown
Member

@felixfontein Is this PR still needed after the deprecation changes in 2.19? If so, can you provide some additional details on what this is fixing?

@felixfontein

felixfontein commented Jun 7, 2025

Copy link
Copy Markdown
Contributor Author

I have no idea, I don't know the changes well enough to be able to say.

@mattclay

mattclay commented Jun 7, 2025

Copy link
Copy Markdown
Member

@felixfontein Do you remember what issue this was meant to resolve?

@felixfontein

felixfontein commented Jun 7, 2025

Copy link
Copy Markdown
Contributor Author

I think it was mainly to make sure that once #73051 was fixed, that the right collection name is shown next to a plugin-related deprecation ("... will be removed from foo.bar 2.0.0").

@Akasurde

Akasurde commented Feb 19, 2026

Copy link
Copy Markdown
Member

Closing this as per #73051 (comment)

@Akasurde Akasurde closed this Feb 19, 2026
@ansible ansible locked and limited conversation to collaborators Mar 19, 2026
@felixfontein
felixfontein deleted the fix-plugin-deprecations-1 branch May 17, 2026 10:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

affects_2.15 affects_2.18 bug This issue/PR relates to a bug. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@felixfontein @mattclay @Akasurde @Shrews @ansibot @nitzmahone