fix(veritech): When errors occur that don't cause us to retry the function, inform subscribers #5263
Conversation
This file contains 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
github-actions
bot
added
the
A-veritech
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
sprutton1
reviewed
Jan 15, 2025
| Ok(function_result) => { | ||
| if let Err(err) = publisher.publish_result(&function_result).await { | ||
| metric!(counter.function_run.action = -1); |
There was a problem hiding this comment.
This is scooped up by the dec call below, yeah?
There was a problem hiding this comment.
yeah were we double decrementing here?
There was a problem hiding this comment.
yeah I think just leftover from pre-refactor
zacharyhamm
previously approved these changes
Jan 15, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
britmyerss
force-pushed
the
brit/bubble-up-veritech-errors
branch
from
e04f617 to
c34536d
Compare
…ction, inform subscribers This change ensures that for all functions types, when errors occur (such as killing a function, or timing out), we inform subscribers of the failure so they can handle them appropriately. The main issue this fixes is during Pinga jobs, if a function times out or is killed, the job will continue to run until it hits its own time out, exacerbating hung change sets
britmyerss
force-pushed
the
brit/bubble-up-veritech-errors
branch
from
c34536d to
746f098
Compare
|
/try |
|
Okay, starting a try! I'll update this comment once it's running...\n |
zacharyhamm
approved these changes
Jan 16, 2025
nickgerace
reviewed
Jan 16, 2025
Comment on lines
+120
to
+123
| // This should be [`FuncRunState::Killed`] | ||
| // but because this is getting bubbled back to the Pinga Job running the | ||
| // action, and the error is now embedded in a successful response, | ||
| // (because we did get a response from Veritech) |
There was a problem hiding this comment.
UGH! So sorry about this... that explains a lot.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change ensures that for all functions types, when errors occur (such as killing a function, or timing out), we inform subscribers of the failure so they can handle them appropriately.
The main issue this fixes is during DVU jobs, if a function times out or is killed, the job will continue to run until it hits its own time out, exacerbating hung change sets
via GIPHY