-
Notifications
You must be signed in to change notification settings - Fork 0
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
send response to manager on computation termination #3
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces enhancements for handling computation stop requests within the ManagerClient. Key changes include a new response message structure for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ManagerClient
participant Service
participant EventPublisher
Client->>ManagerClient: Request StopComputation
ManagerClient->>Service: Process StopComputation
Service-->>ManagerClient: Response (StopComputationResponse)
ManagerClient->>Client: Send Response
alt Success
Service->>EventPublisher: Notify Stop Success
else Failure
Service->>EventPublisher: Notify Stop Failure
end
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Signed-off-by: WashingtonKK <[email protected]>
6a14ac4
to
6266312
Compare
Signed-off-by: WashingtonKK <[email protected]>
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 ignored due to path filters (1)
pkg/manager/manager.pb.go
is excluded by!**/*.pb.go
Files selected for processing (4)
- manager/api/grpc/client.go (1 hunks)
- manager/manager.proto (2 hunks)
- manager/service.go (1 hunks)
- manager/service_test.go (3 hunks)
Additional comments not posted (11)
manager/manager.proto (2)
24-28
: New messageStopComputationResponse
added.The new message structure provides detailed feedback for stopping computations, which is a valuable addition for better communication and error handling.
62-62
: FieldstopComputationRes
added toClientStreamMessage
.The integration of
StopComputationResponse
intoClientStreamMessage
enhances the message framework by allowing clients to receive detailed responses related to computation stops.manager/api/grpc/client.go (4)
76-78
: New response message structure forStopComputation
.The new response message structure for
StopComputation
includes the computation ID, which is crucial for identifying the computation being stopped.
80-87
: Improved error handling forErrNotFound
.The error handling for
ErrNotFound
is well-implemented, ensuring that a clear message is sent back to the client and the computation is marked as not stopped.
88-93
: Streamlined error handling for other errors.The error handling for other errors is consistent and ensures that the client receives a detailed error message.
95-96
: Success response for stopped computation.The response correctly indicates success when a computation is successfully stopped, enhancing the clarity of the communication protocol.
manager/service_test.go (3)
127-130
: Improved test setup with mock components.The introduction of mock components for the provider and VM enhances the test's control over interactions, allowing for more precise validation of behavior.
163-168
: Enhanced context formanagerService
in tests.The inclusion of a logger and events channel in the
managerService
struct provides a more realistic context for testing, improving the test's robustness.
192-195
: Added cleanup step foreventsChan
.The cleanup step for the
eventsChan
ensures test isolation and prevents side effects, which is crucial for reliable test results.manager/service.go (2)
170-170
: Verify the placement of the successdefer
statement.Ensure that the
defer
statement for the success event is placed appropriately to reflect the intended logic. It should be placed after the computation is successfully stopped and removed from the map.
177-177
: LGTM! Use ofdefer
for failure event is appropriate.The
defer
statement for the failure event is well-placed to ensure the event is published if the computation ID is not found.
Signed-off-by: WashingtonKK <[email protected]>
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 ignored due to path filters (1)
pkg/manager/manager.pb.go
is excluded by!**/*.pb.go
Files selected for processing (2)
- manager/api/grpc/client.go (1 hunks)
- manager/manager.proto (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- manager/api/grpc/client.go
- manager/manager.proto
Signed-off-by: WashingtonKK <[email protected]>
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)
- manager/service.go (1 hunks)
- manager/service_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- manager/service.go
- manager/service_test.go
What type of PR is this?
What does this do?
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Did you document any new/modified feature?
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Tests