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.

Sign up for GitHub

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

Jump to bottom

cmdmod.py: Don't reset output vars inside loop #62903

Merged
merged 3 commits into from
Nov 18, 2022
Merged

cmdmod.py: Don't reset output vars inside loop #62903

merged 3 commits into from
Nov 18, 2022

Conversation

roelvanmeer
Copy link
Contributor

@roelvanmeer roelvanmeer commented Oct 18, 2022 •
edited
Loading

The stdout and stderr variables are used to collect the output of the command. These should not be reset inside the loop, because then the output that was already collected gets lost.

This problem was likely introduced by c7d2756, which seems to be a purely formatting commit.

Fixes: #60365

@roelvanmeer
cmdmod.py: Don't reset output vars inside loop
339eea7 
The stdout and stderr variables are used to collect the output of
the command. These should not be reset inside the loop, because then the
output that was already collected gets lost.

Fixes: 60365
@roelvanmeer roelvanmeer requested a review from a team as a code owner October 18, 2022 10:18
@roelvanmeer roelvanmeer requested review from MKLeb and removed request for a team October 18, 2022 10:18
@welcome
Copy link

welcome bot commented Oct 18, 2022

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Ch3LL
Ch3LL suggested changes Oct 18, 2022
View reviewed changes
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

Thanks for this change. This will require a changelog and test coverage.

@roelvanmeer
Copy link
Contributor Author

Can you please label this as Needs Testcase and Help Wanted then, since I won't have time to figure out how to do those any time soon. Thanks!

@Ch3LL Ch3LL added Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases help-wanted Community help is needed to resolve this labels Oct 18, 2022
@Ch3LL Ch3LL added Sulfur v3006.0 release code name and version and removed help-wanted Community help is needed to resolve this Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases labels Nov 17, 2022
@Ch3LL Ch3LL merged commit 8a62217 into saltstack:master Nov 18, 2022
@welcome
Copy link

welcome bot commented Nov 18, 2022

Congratulations on your first PR being merged! 🎉

@whytewolf whytewolf mentioned this pull request Nov 29, 2022
Closed
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ch3LL Ch3LL Ch3LL approved these changes

@twangboy twangboy twangboy approved these changes

@MKLeb MKLeb Awaiting requested review from MKLeb MKLeb was automatically assigned from saltstack/team-core

Assignees
No one assigned
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Minions running 3002+ throw a JSONDecodeError when called from master
4 participants
@roelvanmeer @Ch3LL @twangboy @MKLeb