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 task.resolved_action callbacks #82003

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

s-hertel
Copy link
Contributor

SUMMARY

Noticed while looking at the related fix to not use this attribute for module_defaults in #81909.

Not sure if there is a better way to set this - it feels a bit redundant to load the plugin context additional times.

ISSUE TYPE
  • Bugfix Pull Request

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Oct 17, 2023
@s-hertel
Copy link
Contributor Author

@nitzmahone Not sure if you have any thoughts on this, but after thinking about this in conjunction with module_defaults, I'm wondering if the resolved_action attribute is just rather unhelpful for collections that use a common action plugin.

For example, say vyos_mod1 and vyos_mod2 are redirected to the action plugin vyos. To prevent module_defaults for vyos_mod1 clobbering the default args for vyos_mod2 or vice versa, they can add the action_plugin directive for modules in the runtime.yml.

If these tasks run while using a callback that displays the resolved_action, they will all be displayed as the vyos action, rather than the potentially-more-helpful module name. This is only sort of similar to the module_defaults issue (basically unusable feature vs bad readability). Could I adjust the resolved_action to reflect the resolved module in this case, or is that overloading the task attr/the module metadata magic?

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Oct 17, 2023
@s-hertel s-hertel force-pushed the fix-resolved-action-callbacks branch 2 times, most recently from 7c470cc to 161204a Compare October 17, 2023 20:30
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Oct 18, 2023
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Oct 19, 2023
@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 2, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jan 9, 2024
module invocations, also update unit tests to use plugins which exist

Or I could wrap the new validation in a try/except ImportError to
support the previous behavior for missing plugins?
@s-hertel s-hertel force-pushed the fix-resolved-action-callbacks branch from 43ee751 to 3ae00d6 Compare January 31, 2024 18:39
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jan 31, 2024
@s-hertel
Copy link
Contributor Author

I didn't actually realize this also fixes #81905, but it does so I added tests and a changelog.

@s-hertel s-hertel marked this pull request as ready for review January 31, 2024 18:51
@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 Jan 31, 2024
@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 Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants