Fix minion status tracking for direct state.apply dispatches - #1
Open
avinashbhatv wants to merge 2 commits into
Open
Fix minion status tracking for direct state.apply dispatches#1avinashbhatv wants to merge 2 commits into
avinashbhatv wants to merge 2 commits into
Conversation
dwoz
requested changes
Aug 6, 2026
| if expected_minions: | ||
| try: | ||
| returns_resp = api_client.call("ret", "get_returns", jid=jid) | ||
| except Exception: |
There was a problem hiding this comment.
Is there a specific exception or class of exception we can expect here? In general "bare exception" catching is frowned upon.
Author
There was a problem hiding this comment.
Good catch , it doesnt need to be this broad. api_client.call() raises salt_config_cli.api.exceptions.APIError (or one of its subclasses : ConnectionError, TimeoutError, AuthenticationError, ServerError) for anything RPC-related. I'll narrow it to:
except APIError:
Per review feedback on PR saltstack#1: api_client.call() raises salt_config_cli.api.exceptions.APIError (or a subclass -- ConnectionError, TimeoutError, AuthenticationError, ServerError) for anything RPC-related. These polling checks are best-effort/supplementary (the loop falls back to its primary completion signal either way), so catching the client's own known error types is correct, but a bare except Exception was needlessly broad and would also swallow real bugs (e.g. a TypeError from a malformed response) silently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports commit 462f8f14f from the internal fork.
Fixes
scc run/scc deploy/scc job-statushanging or reporting stale status for jids dispatched via a directcmd.route_cmdcall (not a tracked RaaS Job entity) —cmd.get_cmd_statusnever reports "complete" for those, so:_wait_for_job/_wait_for_job_livenow also complete onceret.get_returnshas a result for every targeted minion (when the target is alist-type target with a known member count), independent of the RaaS job-status flag.job-statusfalls back toret.get_returnswhen the status registry reports something unrecognized (e.g.not-found) instead of giving up.Verified against a live RaaS/vCenter environment: both dry-run and apply now return control immediately after the minion reports, instead of hanging until the 1800s timeout.