feat(controller) Add and scaffold Cancel API in gateway#114
Conversation
manjari25
force-pushed
the
manjari/cancel-gateway
branch
from
c365d38 to
fb87fce
Compare
manjari25
requested review from
a team,
behinddwalls and
sbalabanov
as code owners
manjari25
force-pushed
the
manjari/cancel-gateway
branch
from
1288676 to
ff0ef20
Compare
| // Create queue message | ||
| // - Message ID: sqid for idempotency | ||
| // - Payload: sqid as bytes | ||
| // - Partition key: sqid (ensures ordering per request) |
There was a problem hiding this comment.
how would it be ordered?
what if 2 sqids are being cancelled at the same time: "go/2" and "go/10"
There was a problem hiding this comment.
On second thought, I am not sure if we care about ordering for cancellation. I don't think there is any inherent order we need to enforce. We can simply leave the partition key empty. Thoughts?
There was a problem hiding this comment.
I think order does not matter here
| sqid := req.Sqid | ||
|
|
||
| if sqid == "" { | ||
| return &pb.CancelResponse{ |
There was a problem hiding this comment.
it should be INVALID_ARGUMENT + BadMessage first class GRPC error
errdetails.BadRequest
code.InvalidArgument
import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/genproto/googleapis/rpc/errdetails"
)
There was a problem hiding this comment.
The transformation from Go errors to GRPC errors is better to be done at a certain distinct function, means Cancel() function likely wants to wrap another cancelRequest(...) error function which is to return first class validationError.
|
|
||
| // Cancel represents a request to cancel a previously submitted land request. | ||
| // The object is immutable after creation. | ||
| type Cancel struct { |
| // Current status of the land request at the time of cancellation request. | ||
| RequestStatus current_status = 2; | ||
| // Optional error describing why the cancellation could not be completed. | ||
| Error error = 3; |
There was a problem hiding this comment.
should be first class grpc error
| // processing. Note that cancellation is best effort and not guaranteed. | ||
| // To check for the status of the request, use Status API (TODO). A | ||
| // "Cancelled" status indicates that the requested cancellation was | ||
| // successful. |
There was a problem hiding this comment.
may be update the last sentence to describe that it returns the current status, whatever it is
| // Cancel handles the cancel request and returns a response. | ||
| // It publishes the sqid to the cancel topic for async processing. | ||
| // No validation is performed — the orchestrator handles that. | ||
| func (c *CancelController) Cancel(ctx context.Context, req *pb.CancelRequest) (resp *pb.CancelResponse, retErr error) { |
There was a problem hiding this comment.
Note to future us (non blocking): we should discuss if controllers should receive proto structures directly. It seems that the final surfacing of the error should be a responsibility of a service, not a controller.
| // The request has been successfully processed and landed. Terminal state. | ||
| LANDED = 4; | ||
| // The cancellation request has been accepted and is being processed. | ||
| CANCELLATION_ACCEPTED = 5; |
There was a problem hiding this comment.
may be just "cancelling"
3 participants
Summary
feat(controller) Add and scaffold Cancel API in gateway
What?
Why?
Test Plan
Issues