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 use of random shuffle and sample functions as Jinja filters #62373

Merged
merged 7 commits into from
Aug 2, 2022

Conversation

nicholasmhughes
Copy link
Collaborator

What does this PR do?

See issue for details

What issues does this PR fix or reference?

Fixes: #62372

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@nicholasmhughes nicholasmhughes requested a review from a team as a code owner July 25, 2022 14:42
@nicholasmhughes nicholasmhughes requested review from Ch3LL and removed request for a team July 25, 2022 14:42
@nicholasmhughes
Copy link
Collaborator Author

@Ch3LL , will this get back-ported to master or do I need to get another PR in?

@nicholasmhughes
Copy link
Collaborator Author

re-run pr-centosstream-9-x86_64-py3-pytest

@Ch3LL Ch3LL added the Phosphorus v3005.0 Release code name and version label Jul 25, 2022
@whytewolf
Copy link
Collaborator

hey @nicholasmhughes we would rather not push more includes as a work around. if the functions are only meant as jinja filters they should be added/moved to salt/utils/jinja.py.

Also thank you for all the PR's you have been submitting.

@nicholasmhughes
Copy link
Collaborator Author

@whytewolf , near as I can tell this is the intended operation for including the decorator and not a workaround. As an example, the salt.utils.network module is imported in that file but not used at all.

@whytewolf
Copy link
Collaborator

@nicholasmhughes I would be much more comfortable if the item being included in the render state wasn't coming from the modules. there are a couple of minor exceptions. modules that existed before utils was a thing like cmdmod and selinux. but overall importing from modules into utils is bad.

Also. from the original discussion about the decorator it isn't working as expected. it shouldn't need to be imported directly if it was working right.

@Ch3LL Ch3LL merged commit ca73e24 into saltstack:freeze Aug 2, 2022
@nicholasmhughes nicholasmhughes deleted the fix-random-filters branch August 2, 2022 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants