Skip to content

Fix Salt Syndic return events missing fun_args #62809

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

Merged
merged 4 commits into from
Nov 8, 2022

Conversation

keslerm
Copy link
Contributor

@keslerm keslerm commented Oct 4, 2022

This fixes issue #45823 which is the fun_args value missing from the Salt Syndic returns.

What does this PR do?

This includes the fun_args value from the Syndic return. Currently this is missing resulting in the fun_args value being null.

What issues does this PR fix or reference?

Fixes: #45823

Previous Behavior

fun_args is null in the return

New Behavior

fun_args is populated with the correct values

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.

@keslerm keslerm requested a review from a team as a code owner October 4, 2022 21:58
@keslerm keslerm requested review from MKLeb and removed request for a team October 4, 2022 21:58
@keslerm keslerm force-pushed the fix-fun_args-syndic branch from 2fa134d to aa0dc76 Compare October 5, 2022 18:53
@MKLeb
Copy link
Contributor

MKLeb commented Oct 5, 2022

Hi @keslerm and thanks for your contribution. This will need some tests in order to get merged, and if you need help with that or a direction to go in then I will be glad to help.

@MKLeb MKLeb added Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases needs-changelog-file labels Oct 10, 2022
@MKLeb
Copy link
Contributor

MKLeb commented Oct 17, 2022

@keslerm Any updates here?

@keslerm
Copy link
Contributor Author

keslerm commented Oct 17, 2022

@MKLeb hey yeah @waynew helped me get started but i got tied up with some things last week and was aiming to crack on adding the tests this week.

@MKLeb
Copy link
Contributor

MKLeb commented Oct 18, 2022

@MKLeb hey yeah @waynew helped me get started but i got tied up with some things last week and was aiming to crack on adding the tests this week.

Awesome, thank you!!

@keslerm keslerm force-pushed the fix-fun_args-syndic branch from aa0dc76 to a565802 Compare November 1, 2022 18:44
@MKLeb
Copy link
Contributor

MKLeb commented Nov 4, 2022

@keslerm, any luck at writing tests for this? I can help if needed!

This fixes issue saltstack#45823 which is the fun_args value missing from the
Salt Syndic returns.
@keslerm keslerm force-pushed the fix-fun_args-syndic branch from a565802 to e1f4863 Compare November 7, 2022 14:45
@keslerm
Copy link
Contributor Author

keslerm commented Nov 7, 2022

@MKLeb Took a first stab at a test

@keslerm
Copy link
Contributor Author

keslerm commented Nov 7, 2022

@MKLeb I added one other change which is testing if the master_job_cache is false or not since testing it here wasn't part of the objective and the documentation states

However, you can disable the job_cache by setting it to False in the Salt Master configuration file

@MKLeb
Copy link
Contributor

MKLeb commented Nov 7, 2022

re-run pr-amazon-2-x86_64-py3-pytest
re-run pr-ubuntu-2204-amd64-py3-m2crypto-pytest

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

👍

@garethgreenaway garethgreenaway merged commit d75e467 into saltstack:master Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fun_args not passed properly via salt-syndic
5 participants