Discuss: namespace rules prototype #550
Conversation
ychebotarev
force-pushed
the
y_proto_defs
branch
from
5345eca to
ddff8dc
Compare
Ah, for some reason I wasn't getting notifications on all of the comments, I missed a couple of days worth of discussions. |
| // - BackoffInterval: The current backoff interval of the activity. | ||
| // - ActivityStatus: The status of the activity. Can be one of "scheduled", "started", "paused". | ||
| // - TaskQueue: The name of the task queue of the activity. | ||
| string predicate = 1; |
There was a problem hiding this comment.
May need to say this takes the same format as visibility queries just with different attributes. @rodrigozhou - now that we're reusing this format, can we now publish the grammar doc that we maintain internally?
ychebotarev
force-pushed
the
y_proto_defs
branch
from
adcf953 to
ec5a7e4
Compare
| message ActionPause { | ||
| // Indicates what should be paused. Depending on the scope value, the action | ||
| // will pause the entire workflow or a specific activity (additional entities | ||
| // may be supported in the future). | ||
| temporal.api.enums.v1.RuleActionScope scope = 1; | ||
| } |
There was a problem hiding this comment.
I am not sure it makes sense to model activity pause and workflow pause together. What do they have in common besides the word "pause"? I think these are two separate actions, I think only activity pause should be implemented now, and I don't think the "action scope" should be a thing. Sooner or later you'll want an action-pause-only or workflow-pause-only specific option and regret that you combined them into the same structure just because they both have the word "pause".
There was a problem hiding this comment.
there are two ways of modeling this - have a separate message to encode each valid <action, scope> pair:
message ActivityPauseAction {
}
message WorkflowPauseAction {
}
...
or encapsulation - single action - pause - that can be applied in different scope.
message ActionPause {
Scope scope;
}
There are multiple pro-con for each of those approaches, but in a context of current WorkflowRule and future rules (according to Roey they will come) generic action with specific scope feels like a cleaner fit.
Yes, its a valid concern, that sooner or later I would have some action that is not applicable to all scopes, like "activity-specific" action. It will be a situation where not every pair of * is supported. "pro" (at least for me) are outweighing "con".
Specifically:
- less upfront complexity. If we decide to encode every cross-product as a single message it will make a bigger surface. For 4 actions 2 scopes it is already 8 different messages. And it is very likely there will be 4.Messy.
- Extensibility. We expect more "scope" and "actions". and adding new will be easier this way.
- This is "reactive' API, so compile-time guarantee (main "pro" of separate messages for every pair) is not that important. Run-time validation is good enough.
| } | ||
|
|
||
| // Emit a metric. | ||
| message ActionEmitMetric { |
There was a problem hiding this comment.
Is this part of the project expected to be implemented now? If so, can I get more details on what metric is emitted and options and such?
There was a problem hiding this comment.
it is not expected to be implemented now. It will not be in the "merge" PR
<!-- Describe what has changed in this PR --> **What changed?** Adding basic workflow rules operations: * CreateWorkflowRule * DescribeWorkflowRule * DeleteWorkflowRule * ListWorkflowRules <!-- Tell your future self why have you made these changes --> **Why?** Part of workflow rules workstream. Discussed in [this PR](#550).
What changed?
Not for merge
This is a proposal for "namespace level rules". First implementation will be "activity pause".
This PR is based in the latest discussion, and contains all the changes.
Old PR:
#532