Skip to content

Conversation

Shivs11
Copy link
Member

@Shivs11 Shivs11 commented May 25, 2025

What changed?

  • WISOTT

Why?

  • Versioning-0.32

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

  • None

@@ -1079,7 +1079,7 @@ func (d *ClientImpl) updateWithStartWorkerDeploymentVersion(
BuildId: buildID,
},
CreateTime: now,
RoutingUpdateTime: now,
RoutingUpdateTime: nil,
Copy link
Member Author

Choose a reason for hiding this comment

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

this was wrong! we should never set the routingUpdateTime when starting a version workflow

@@ -984,47 +987,6 @@ func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_InvalidSetRampingV
s.EqualErrorf(err, workerdeployment.ErrRampingVersionDoesNotHaveAllTaskQueues, err.Error())
}

func (s *DeploymentVersionSuite) setRamping(
Copy link
Member Author

Choose a reason for hiding this comment

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

don't worry - just moved these helpers to the bottom of the file so they all look nice together

@Shivs11 Shivs11 marked this pull request as ready for review May 28, 2025 15:34
@Shivs11 Shivs11 requested a review from a team as a code owner May 28, 2025 15:34
Shivs11 added a commit to temporalio/api that referenced this pull request May 28, 2025
_**READ BEFORE MERGING:** All PRs require approval by both Server AND
SDK teams before merging! This is why the number of required approvals
is "2" and not "1"--two reviewers from the same team is NOT sufficient.
If your PR is not approved by someone in BOTH teams, it may be summarily
reverted._

<!-- Describe what has changed in this PR -->
**What changed?**
- add VersionStatus inside `WorkerDeploymentVersionInfo` +
`WorkerDeploymentVersionSummary`


<!-- Tell your future self why have you made these changes -->
**Why?**
- right now, the UI code deduces the status of a version by checking
`CurrentSince`, `RampingSince` and other attributes. This can just be
simplified by adding this enum and the server doing the heavy lifting
instead.


<!-- Are there any breaking changes on binary or code level? -->
**Breaking changes**
- None


<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**
- [741011](temporalio/temporal#7804)
temporal-cicd bot pushed a commit to temporalio/api-go that referenced this pull request May 28, 2025
_**READ BEFORE MERGING:** All PRs require approval by both Server AND
SDK teams before merging! This is why the number of required approvals
is "2" and not "1"--two reviewers from the same team is NOT sufficient.
If your PR is not approved by someone in BOTH teams, it may be summarily
reverted._

<!-- Describe what has changed in this PR -->
**What changed?**
- add VersionStatus inside `WorkerDeploymentVersionInfo` +
`WorkerDeploymentVersionSummary`

<!-- Tell your future self why have you made these changes -->
**Why?**
- right now, the UI code deduces the status of a version by checking
`CurrentSince`, `RampingSince` and other attributes. This can just be
simplified by adding this enum and the server doing the heavy lifting
instead.

<!-- Are there any breaking changes on binary or code level? -->
**Breaking changes**
- None

<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**
- [741011](temporalio/temporal#7804)
@Shivs11 Shivs11 merged commit 4d4f069 into main May 28, 2025
56 checks passed
@Shivs11 Shivs11 deleted the ss/version_status branch May 28, 2025 18:55
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.

2 participants