-
Notifications
You must be signed in to change notification settings - Fork 3k
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
lambda_manager: revamp function parsing error handling #8406
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update enhances the error handling for lambda function endpoints by changing the response for invalid metadata from a 404 to a 500 error. It introduces a new validation step for lambda function types and implements a new exception class, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LambdaFunction
participant ServerLogManager
User->>LambdaFunction: Request list of functions
LambdaFunction->>LambdaFunction: Validate function metadata
alt Invalid metadata
LambdaFunction->>ServerLogManager: Log error
LambdaFunction-->>User: Return 500 error
else Valid metadata
LambdaFunction-->>User: Return list of functions
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This has several goals: * Remove `LambdaType.UNKNOWN`. Functions with this non-type are useless and should not be presented to the user. * Don't return 404 from the list endpoint if one function cannot be loaded. This prevents one bad function from essentially disabling the entire serverless function feature. Instead, log the error and ignore the function. * Don't return 404 from other endpoints either when the problem is a bad function. This is not a client problem. Raise an exception and let Django log it and return a 500. * Remove HTTP codes from `LambdaFunction`, to improve separation of concerns.
e6b84ba
to
746c49a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- changelog.d/20240905_201903_roman_lambda_error_handling.md (1 hunks)
- cvat/apps/lambda_manager/views.py (7 hunks)
Additional comments not posted (8)
changelog.d/20240905_201903_roman_lambda_error_handling.md (3)
3-5
: Changelog entry is clear and accurate.This entry correctly summarizes the changes made to the error handling in lambda function endpoints, aligning with the PR objectives.
7-9
: Changelog entry is clear and accurate.This entry effectively communicates the removal of
LambdaType.UNKNOWN
and its implications for the list endpoint, which is a key objective of the PR.
13-15
: Changelog entry is clear and accurate.This entry correctly describes the fix to prevent a single invalid lambda function from disrupting the entire function listing, aligning with the PR's error handling improvements.
cvat/apps/lambda_manager/views.py (5)
51-57
: Confirmed Removal ofLambdaType.UNKNOWN
.The enumeration
LambdaType
no longer includesUNKNOWN
, which aligns with the PR objectives to improve clarity and prevent the use of undefined lambda function types.
97-101
: Improved Error Handling inlist
Method.The modification to yield instances instead of returning a list enhances memory efficiency. The error handling by logging and continuing processing when encountering
InvalidFunctionMetadataError
is a robust approach, ensuring that one faulty function does not affect the availability of others.
138-140
: Introduction ofInvalidFunctionMetadataError
.The new exception class
InvalidFunctionMetadataError
is a welcome addition for clearer and more specific error reporting when handling lambda function metadata issues.
Line range hint
156-202
: Enhanced Error Handling inLambdaFunction
.The modifications in the constructor and
parse_labels
method of theLambdaFunction
class to useInvalidFunctionMetadataError
enhance the clarity and consistency of error handling. This change ensures that errors are more descriptive and aligned with the types of issues encountered during lambda function instantiation.
138-140
: Review ofLambdaQueue
.The
LambdaQueue
class appears unchanged in this PR. It's important to ensure that its functionality aligns with the overall changes in error handling and lambda function management. No issues are found with the current implementation in the context of this PR.
Quality Gate passedIssues Measures |
This has several goals: * Remove `LambdaType.UNKNOWN`. Functions with this non-type are useless and should not be presented to the user. * Don't return 404 from the list endpoint if one function cannot be loaded. This prevents one bad function from essentially disabling the entire serverless function feature. Instead, log the error and ignore the function. * Don't return 404 from other endpoints either when the problem is a bad function. This is not a client problem. Raise an exception and let Django log it and return a 500. * Remove HTTP codes from `LambdaFunction`, to improve separation of concerns.
Motivation and context
This has several goals:
Remove
LambdaType.UNKNOWN
. Functions with this non-type are useless and should not be presented to the user.Don't return 404 from the list endpoint if one function cannot be loaded. This prevents one bad function from essentially disabling the entire serverless function feature. Instead, log the error and ignore the function.
Don't return 404 from other endpoints either when the problem is a bad function. This is not a client problem. Raise an exception and let Django log it and return a 500.
Remove HTTP codes from
LambdaFunction
, to improve separation of concerns.How has this been tested?
Manual testing.
Checklist
develop
branch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have linked related issues (see GitHub docs)[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation