Conversation
go.mod
Outdated
| // Set to released version once available | ||
| go.temporal.io/sdk v1.34.1-0.20250609225810-918fea843587 |
There was a problem hiding this comment.
We'll need to use the newest SDK release before merging next-server into main
There was a problem hiding this comment.
Usually we don't even want unreleased dependencies in next-server. Can this PR wait to be merged until the SDK has been released?
Sushisource
force-pushed
the
versioning-refactor
branch
2 times, most recently
from
98d2cfb to
d53a50c
Compare
Sushisource
force-pushed
the
versioning-refactor
branch
from
d53a50c to
f337211
Compare
cretz
left a comment
cretz
left a comment
There was a problem hiding this comment.
LGTM, but would rather not merge even to next-server until dependency tagged unless there is something urgent requiring it be usable in CLI before even usable in SDK.
go.mod
Outdated
| // Set to released version once available | ||
| go.temporal.io/sdk v1.34.1-0.20250609225810-918fea843587 |
There was a problem hiding this comment.
Usually we don't even want unreleased dependencies in next-server. Can this PR wait to be merged until the SDK has been released?
Sure, we can wait until SDK is released. Some non-Go SDKs already have this released |
temporalcli/commandsgen/commands.yml
Outdated
| (Only for pinned). | ||
| - name: versioning-override-build-id | ||
| type: string | ||
| description: Build Id component of override Pinned Version for a Worker Deployment |
There was a problem hiding this comment.
This description could use a de-mangling.
How about
Override the build ID to move your running Workflow to another build within its Worker Deployment. The workflow must already be pinned or you must also pass --versioning-override-behavior pinned
There was a problem hiding this comment.
Well, that isn't quite right. You can only use this option if you pass pinned. I'll rephrase it though.
| - pinned | ||
| - auto_upgrade | ||
| - name: versioning-override-pinned-version | ||
| - name: versioning-override-deployment-name |
There was a problem hiding this comment.
Suggest: Override when you want to move your running Workflow to a different Worker Deployment. The workflow must already be pinned or you must also pass --versioning-override-behavior pinned
There was a problem hiding this comment.
Is it allowable to just pass in the build ID without this one if they're not changing deployments? (I hope so.) I didn't see a test for that case but forgive me if I missed it.
There was a problem hiding this comment.
I don't know if server will accept that. I can try.
I added a test for this. Server accepts it, but the override doesn't include the deployment name any more afterward (it does include the non-override deployment name, which is the same, so maybe that's fine).
| // TODO: Unclear why this just is not showing up in the response | ||
| // require.NotNil(t, versioningInfo.VersioningOverride) | ||
| // TODO: Fix | ||
| // require.Equal(t, version2, versioningInfo.VersioningOverride.PinnedVersion) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Carly told me this is a bug they've discovered in server
There was a problem hiding this comment.
Can confirm this is fixed now
Co-authored-by: Carly de Frondeville <[email protected]>
4 participants
What was changed
Refactor versioning to use latest SDK & avoid string-based deployment versions
Why?
Consistent with SDK refactorings
Checklist
Closes [CLI] Update protos, use buildid/deployment name args & support versioning overrides when starting workflows #804
How was this tested:
Existing tests
Any docs updates needed?