Add flags for using the default build id set #273
Conversation
There was a problem hiding this comment.
Overall LGTM.
I'm just not convinced that for continue-as-new (and maybe even child workflow) we should stay within the compatible set by default.
It's of course a tradeoff between safety and being able to retire workers.
I have a feeling some users will be surprised by these defaults but I'm okay with starting out on the safer side.
I do think we should at least discuss this a bit more before making a call because up until recently the understanding was that CAN will go to the default since the expectation is that workflows are external APIs and cannot arbitrarily change their signatures, that's of course case dependent, and some workflows are public while others are private but we don't have this concept in the SDKs.
|
@bergundy The other two, I have fairly high confidence that's the right choice there. |
|
Oh and you're going to want to propagate this intent to the relevant history events. |
|
I have this thought that maybe if we see that a workflow was started within a compatible set, when it continues as new, it stays within that set. |
There was a problem hiding this comment.
shouldn't this go to the feature branch instead of master?
I think my leanings are:
- CAN should go to the same set by default if the task queue and workflow type are the same. that feels very "private". the typical use for that kind of CAN is to make an infinite loop (given our limitations). it would be surprising to me if it started running some other code on another binary.
- if the task queue is different I agree it should go to the overall default.
- if the task queue is the same but the workflow type is different... I don't know.
- whatever the behavior, it seems easier to explain/learn if all three of these are consistent
| // is the default behavior) and instead will use the current overall default for the queue. | ||
| // If this command's `task_queue` field differs from the executing workflow's task queue, then | ||
| // this flag has no effect. | ||
| bool use_default_build_id_set = 13; |
There was a problem hiding this comment.
not sure about the name.. "build id set" isn't really what users are thinking in terms of, right?
| bool use_default_build_id_set = 13; | |
| bool use_latest_version = 13; |
?
| bool use_default_build_id_set = 13; | |
| bool use_latest_build_id = 13; |
?
There was a problem hiding this comment.
use_latest_build_id is probably better
By that logic, it would always continue within the set. The only case a workflow starts without being pinned to some set is if it started before there were any versions, and then we add one. Otherwise, the workflow's version set is determined once its first task starts. Alternatively, you could interpret that as they never start with some defined set. The only case where it seems clear they are predestined for some specific set is when they're run as child workflows, for example - if that's what you're getting at? I think there's merit to both approaches for CAN. The above feels much too annoying to explain. I'm inclined to say they should start on the same compat set for two reasons:
|
What changed?
Add flags to activity/child/c-a-n to explicitly use the default version set rather than the compatible set when sticking to the same queue.
Why?
Flexibility with the versioning feature
Breaking changes