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

command warnings: don't send the param from action #70531

Merged
merged 1 commit into from Jul 9, 2020

Conversation

relrod
Copy link
Member

@relrod relrod commented Jul 8, 2020

SUMMARY

Change:

  • Followup to Deprecate command warnings feature #70504. We need to not pass the 'warn' parameter from the
    action plugin either, unless it's True. Otherwise, even though it
    defaults to false, we always show the deprecation.

Test Plan:

  • Local

Signed-off-by: Rick Elrod rick@elrod.me

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

command, shell

@relrod relrod requested review from bcoca and nitzmahone July 8, 2020 22:15
@ansibot ansibot added affects_2.11 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 8, 2020
@relrod relrod requested a review from samdoran July 9, 2020 15:39
Change:
- Followup to ansible#70504. We need to not pass the 'warn' parameter from the
  action plugin either, unless it's True. Otherwise, even though it
  defaults to false, we always show the deprecation.

Test Plan:
- Local

Signed-off-by: Rick Elrod <rick@elrod.me>
@relrod relrod force-pushed the modules/command/actionfix branch from 75b734f to 38de567 Compare July 9, 2020 16:12
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed small_patch labels Jul 9, 2020
@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 In order to be merged, this PR must follow the core review workflow. labels Jul 9, 2020
@samdoran samdoran merged commit 28fda23 into ansible:devel Jul 9, 2020
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Jul 9, 2020
@@ -17,7 +17,7 @@ def run(self, tmp=None, task_vars=None):
del tmp # tmp no longer has any effect

# Command module has a special config option to turn off the command nanny warnings
if 'warn' not in self._task.args:
if 'warn' not in self._task.args and C.COMMAND_WARNINGS:
Copy link
Member

Choose a reason for hiding this comment

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

this changes the meaning, before we only set the configured default if it was not set in the task itself, now we only set it if the configured default is true.

what you really want is to set it if the configured default is explicit and not from 'default', so those setting it to 'false' in the configuration should also get a warning. (use get_config_value_and_origin to get both value and where it was set from)

@ansible ansible locked and limited conversation to collaborators Aug 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.11 bug This issue/PR relates to a bug. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants