Skip to content

Conversation

@ShahabT
Copy link
Contributor

@ShahabT ShahabT commented May 26, 2025

What changed?

Change Deployment Version SA delimiter from . to :.

Why?

New delimiter is used in:

  • TemporalWorkerDeploymentVersion SA
  • BuildIds SA, for the pinned:<deployment name>:<build id> format
  • Deployment Version workflow ID

Old delimiter is used in:

  • v31 version string values in external API
  • version string values in internal entity workflows and APIs

We'll refactor code to clean up usage of old delimiter and string version fields later. For now, the priority is to make public preview external APIs consistent.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

If Versioning V0.31 pre-release users directly use these SAs, they wont be able to find old or new wfs if they filter using only one of the delimiters. (This affects "Go to Workflow" links in UI until UI is updated)

@ShahabT ShahabT requested a review from carlydf May 26, 2025 07:45
@ShahabT ShahabT requested a review from a team as a code owner May 26, 2025 07:45
@ShahabT ShahabT force-pushed the shahab/version-delim branch from f4b1db6 to 58b3644 Compare May 26, 2025 08:23
@ShahabT ShahabT force-pushed the shahab/can-version branch from 8443fec to 251d17f Compare May 26, 2025 08:36
@ShahabT ShahabT force-pushed the shahab/version-delim branch from 58b3644 to e75fbb0 Compare May 26, 2025 08:37
Copy link
Member

@Shivs11 Shivs11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm except the test change which I am not sure if you meant to include here or not

Comment on lines 38 to 42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// WorkerDeploymentVersionIdDelimiterV31 will be deleted once we stop supporting v32 version string fields
// WorkerDeploymentVersionIdDelimiterV31 will be deleted once we stop supporting v31 version string fields

(nit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did u mean to have this change here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ShahabT ShahabT force-pushed the shahab/version-delim branch 2 times, most recently from 6e3fea9 to 94ca0d3 Compare May 27, 2025 02:46
Base automatically changed from shahab/can-version to main May 27, 2025 12:05
@ShahabT ShahabT force-pushed the shahab/version-delim branch from 2d2a50c to f19170a Compare May 27, 2025 21:23
@ShahabT ShahabT merged commit 1f75c8e into main May 30, 2025
56 checks passed
@ShahabT ShahabT deleted the shahab/version-delim branch May 30, 2025 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants