Add a new AgentError field in the agent protos #5916
Merged
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
Signed-off-by: Rafael Raposo <[email protected]>
RRap0so
force-pushed
the
rafaelraposo/add-custom-error-handling
branch
from
73bf769
to
45b2f7c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5916 +/- ##
==========================================
- Coverage 37.03% 37.02% -0.02%
==========================================
Files 1313 1313
Lines 131622 131691 +69
==========================================
+ Hits 48742 48753 +11
- Misses 78652 78710 +58
Partials 4228 4228
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Signed-off-by: Rafael Raposo <[email protected]>
Signed-off-by: Rafael Raposo <[email protected]>
Signed-off-by: Rafael Raposo <[email protected]>
RRap0so
changed the title
Comment on lines
+188
to
189
| logger.Errorf(ctx, "failed to receive from server %s", err.Error()) | ||
| return nil, nil, err |
There was a problem hiding this comment.
this is great, even if this IDL change is not accepted, we should still accept this.
There was a problem hiding this comment.
It overall looks super great, just need @EngHabu for approving this.
Signed-off-by: Rafael Raposo <[email protected]>
Signed-off-by: Rafael Raposo <[email protected]>
Future-Outlier
approved these changes
Nov 22, 2024
pingsutw
approved these changes
Nov 22, 2024
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.
Why are the changes needed?
This aligns Agent Errors with all other Flyte Errors, meaning that similarly to
ContainerErrorwe now haveAgentError. It was decided to use a newAgentErrordue to being calls to an Agent Service and not necessarily containers running so it's better to keep it clean.What changes were proposed in this pull request?
Adding a new
AgentErrorImplement the usage of
AgentErrorLittle tweaks and error message improvements
How was this patch tested?
Unit tests added.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link