Repository navigation
Add plumbing to allow ateom to suspend actors - #1779
Conversation
| if errs := validateActorRef(req.GetActor(), field.NewPath("actor")); len(errs) > 0 { | ||
| return nil, status.Errorf(codes.InvalidArgument, "invalid actor: %v", errs.ToAggregate()) | ||
| } | ||
| if req.GetActorUid() == "" { |
There was a problem hiding this comment.
Should we use the generated DV validations instead? Can we follow the same pattern we follow in the control API of having a validate method that calls the generated DV code and call it from here?
There was a problem hiding this comment.
I don't want workerserve to depend on controlapi which is where the generated DV code currently lives. We should probably move the generated DV code to a neutral apivalidation package which can be imported into both api surfaces.
This would be a pretty significant refactor, can we do this as a follow up PR?
There was a problem hiding this comment.
Agree. We can do it in a follow up. I wanted to move the validations to a separate package (under cmd/ateapi/internal) for other reasons.
There was a problem hiding this comment.
Maybe add a TODO comment in the code to remove the hand-written validations and use DV?
There was a problem hiding this comment.
Done
| // SetWorkerCapacity, naming the Worker its caller proved itself to be. The | ||
| // control plane serves it only for an Actor that is assigned to that Worker. | ||
| // | ||
| // Idempotent in the same sense SuspendActor is: the workflow fast-forwards |
There was a problem hiding this comment.
Is this true? It looks like if the actor is suspended, the method returns NOT_FOUND.
There was a problem hiding this comment.
Updated the comment to reflect the state of the code. We don't want this method to be idempotent as:
- An already suspended actor no longer belongs to this worker, and this worker shouldn't be able to know it exists or make calls against it.
- When moving to FGA for validation, depending on the actor state will make things more complicated.
635028e to
c5fd2c4
Compare
c5fd2c4 to
ce0fc5c
Compare
Part of #483