Add deployment version features #611
Conversation
There was a problem hiding this comment.
IMO the generic helpers here should be in the harness and the workflows should be in the feature files themselves (yes even if it means the few lines are copied across them). For the most part we've tried to keep features self contained.
There was a problem hiding this comment.
I was following build_id_versioning/common.go. Coping the workflows everywhere is not an issue (just a few lines), but I wonder if functions like setCurrent that are very deployment specific belong to the harness...
There was a problem hiding this comment.
Coping the workflows everywhere is not an issue (just a few lines)
👍
I wonder if functions like setCurrent that are very deployment specific belong to the harness...
It seems like normal client use just like our users are expected to use. Is this how we expect our users to use the deployment client (i.e. wait until the server reports it at a certain value before making the next call)?
There was a problem hiding this comment.
Unfortunately, the server API is eventually consistent because we discover worker properties, as opposed to using a declarative approach... This is something I deeply dislike, but after many team discussions It eventually went that way..
In non-test scenarios it may not be that bad, you may only need to check for deployments/versions existence once, since deployments are explicitly deleted and versions are GC slowly. Or use an infinite loop (k8) to change desired state, ignoring early failures.
Back to the issue, is it ok to pollute handler code with all this stuff only applicable to deployment features?
There was a problem hiding this comment.
Back to the issue, is it ok to pollute handler code with all this stuff only applicable to deployment features?
I think it's probably ok (so long as well qualified), but if you want to leave here in common that's fine. But I do think the workflows should be in their feature files (and I'll mark approved after that).
I do think that you as the first client user of versioning are already seeing difficult user experience for programmatically using versioning APIs. I think that may need to be brought up in a devexp context.
| func SignalAll(r *harness.Runner, ctx context.Context, handles []client.WorkflowRun) error { | ||
| for _, handle := range handles { | ||
| if err := r.Client.SignalWorkflow(ctx, handle.GetID(), handle.GetRunID(), "start-signal", "prefix"); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I think this should be inlined where used since it relies on certain expectations of the workflow shape. It's only used for a single handle anywhere anyways and signalling is not so much of a burden it needs a helper
What was changed
Adding Go features for the new versioning API based on Deployments.
[Feature Request] Add samples using our versioning API #227