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

Report failures and error information from tuned module #60534

Closed
wants to merge 2 commits into from

Conversation

leeclemens
Copy link
Contributor

@leeclemens leeclemens commented Jul 13, 2021

Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

What does this PR do?

Returns cmd results from module for better processing and error handling.

What issues does this PR fix or reference?

Fixes: #60500

Previous Behavior

Errors were ignored and cmd output was ignored, specifically from the state. State was reported as successful/changed in some failed executions.

New Behavior

The module returns the full output from the cmd and modifies it when useful (success with the new expected profile being returned in stdout). Report failed execution as failure back to state.

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.

@leeclemens leeclemens requested a review from a team as a code owner July 13, 2021 00:18
@leeclemens leeclemens requested review from dwoz and removed request for a team July 13, 2021 00:18
Replace stdout in cases of success with known value for state to evaluate.
Update response handling in states.

Fixes saltstack#60500
@leeclemens leeclemens marked this pull request as draft July 13, 2021 01:43
@leeclemens leeclemens marked this pull request as ready for review July 13, 2021 22:25
@dwoz
Copy link
Contributor

dwoz commented Dec 10, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

@dwoz dwoz closed this Dec 10, 2023
@dwoz dwoz added help-wanted Community help is needed to resolve this Abandoned labels Dec 10, 2023
@leeclemens
Copy link
Contributor Author

@dwoz can you clarify what you mean by Abandoned? I don't see any feedback from SaltStack beyond tags and being Approved in 2021.

@leeclemens leeclemens deleted the 60500/tuned branch December 12, 2023 04:03
@dwoz
Copy link
Contributor

dwoz commented Dec 16, 2023

@leeclemens We are making an effort to clean up out-dated PRs. This was closed purely because there was no activity in 2 years. If you want to get it into a release please rebase and re-open the PR. We'll get it in.

@leeclemens
Copy link
Contributor Author

@dwoz thanks for the feedback. I suppose I was confused by the "if they want to see it through to the end in one release cycle" comment since you closing the PR was the first feedback I received on either the Issue or the PR created over 2 years ago. The PR was submitted 3 days after opening the Issue. I also marked this PR for review and the first response was to close it as Abandoned.

@leeclemens
Copy link
Contributor Author

It seems like the rebase issue was that the tests were moved to pytests about 1.5 years after the PR was created. I copied them and submitted #65720 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned help-wanted Community help is needed to resolve this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] tuned state does not report error
2 participants