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

Check if param flag values are defined, not truthiness #366

Conversation

zjleblanc
Copy link
Contributor

Addresses #365 by checking if module parameters which represent flags are defined, preventing evaluation of their truthyness. Current behavior will ignore an incoming value of false and default back to true because of the conditional logic:

Example:
module.params["enabled"] = False
if module.params.get("enabled") evaluates to False, but the intent is to determine if it has been defined, so we want this to evaluate to True.

This PR addresses the same issue for swap_single_source param.

@Alex-Izquierdo
Copy link
Contributor

Thanks @zjleblanc
Actually all the parameters of the module are affected by this issue and probably the rest of modules, so I will prefer to create a PR to fix all of them.

@zjleblanc
Copy link
Contributor Author

@Alex-Izquierdo sounds like a plan. Would you like me to close this PR for now? Or start looking at other params and start adding to it?

@Alex-Izquierdo
Copy link
Contributor

Hi @zjleblanc
Sorry for the late response, I was on holidays.
I've checking it deeply and your PR already addresses all the issues, we don't need more changes. The only thing is that we should test this specific issue. Would you mind to add a new task in https://github.com/ansible/event-driven-ansible/blob/main/tests/integration/targets/activation/tasks/main.yml so we can verify that enabled param is correctly created?

Otherwise I can recreate the PR with the test.

Thank you very much.

@zjleblanc
Copy link
Contributor Author

@Alex-Izquierdo added an assert based on the existing integration tasks - will check back to make sure it passes.

Copy link
Contributor

@Alex-Izquierdo Alex-Izquierdo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much for your contribution @zjleblanc

@Alex-Izquierdo Alex-Izquierdo merged commit b395b94 into ansible:main Dec 18, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants