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

Disable Ansible interpreter warnings #2914

Merged
merged 1 commit into from
Oct 27, 2020
Merged

Disable Ansible interpreter warnings #2914

merged 1 commit into from
Oct 27, 2020

Conversation

ssbarnea
Copy link
Member

Switches Ansible warnings around interpreter being picked to silent in order to avoid noise during testing.

Switches Ansible warnings around interpreter being picked to silent
in order to avoid noise during testing.
@geerlingguy
Copy link
Contributor

I still like to have warnings present, because while like 80% of them are non-actionable, annoying, and IMO should not be warnings, there are 20% or so that help me to proactively change things that need changing for future compatibility.

This should be easy to configure for the end user but by default I think they should be enabled.

@greg-hellings
Copy link
Contributor

I'm in agreement with @geerlingguy. Many of the warnings are overzealous - like the python discovery ones - but I rely on it to find deprecations early and prevent failures preemptively. Every time I move to a new version of Ansible and see new warnings pop up it's a great reminder to run through my roles and update them.

@ssbarnea
Copy link
Member Author

ssbarnea commented Oct 24, 2020

@geerlingguy @greg-hellings Feel free to to use the format review option and request changes if you disagree with the proposal. I do find opposition beneficial to the project.

I am also against disabling Ansible warnings in general and this is why I complained on Ansible issue tracker that they have only one nuclear disable deprecation warnings.

Still, this change is about a very particular warning, the Python interpreter being used. I almost always seen these warnings as useless, as the user does not really care about which interpreter is used as long ansible succeeds. Because molecule is used during testing, it means that core will run with newer interpreter before reaching production. This makes the warning useless, because user will already discover an issue before he goes to production.

This ansible warning is useful for those that do not have tests, as it warns them that in next version of Ansible you may see a behavior change. But when you test your code (molecule!), you will always run the tests with the newer version of Ansible before running the same code in production. This means that displaying this warning in molecule context is just noise.

I would rather see a warning when there is something actionable (a way to address it ), and not when something will happen in the future but you cannot do anything about it now. These warning in particular reminded me of the story about the boy that cried wolf... How i see it: warning presence makes people resistant to them and they will soon ignore all of them.

Having less warnings but all actionable, makes it much easier for people to not ignore them.

Copy link
Contributor

@greg-hellings greg-hellings left a comment

Choose a reason for hiding this comment

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

If we can really nuke just this one pointless warning, then I'm in favor of this change.

@ssbarnea
Copy link
Member Author

If we can really nuke just this one pointless warning, then I'm in favor of this change.

Yes, it is only that particular one. I will still let it wait for more feedback, before merging it (or not).

@geerlingguy
Copy link
Contributor

I'm okay with it, in that case.

@ghost
Copy link

ghost commented Oct 26, 2020

I agree with silencing this particular warning.

@ssbarnea ssbarnea merged commit 01a22bc into master Oct 27, 2020
@ssbarnea ssbarnea deleted the fix/auto_silent branch October 27, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants