Is this a bug? If I override and start sending workflows I would expect to be draining until these workflows finish, so that I don't remove the workers...

This is only a temporary limitation. We'll improve it later, but don't want to spend time on it while working on P0 items because this edge case is not expected to be frequent.

(Note that the limitation is only when the version is already drained and THEN use starts new wfs on it with pinned override. If it's not yet drained and user starts wfs with pinned override, the status will count those wfs.)

I know the linter we use suggests naming enums as xyzState and not xyzStatus, but it seems to me "status" would fit here better because it conveys that this is a read-only value that is only reported to user, as opposed to a state that can be mutable by user directly.

Based on the latest UI work, it seems useful to be able to differentiate between these from the Version POV. So I'm considering to have the following enum instead of the bool:

enum VersionRoutingStatus {
  UNSPECIFIED,
  // No routing status, hence new executions are not routed to this Version (unless they are explicitly pinned to this version).
  NONE,
  // This is the Current Version of the Deployment, hence it receives new executions.
  CURRENT,
  // This is the Ramping Version of the Deployment, hence it receives a share of new executions.
  RAMPING,
}

(maybe we'll add more routing statuses particularly for unversioned mode later when we add support for them, but right now we want to keep those parts out of the API because they are not implemented)

In the framework of State = writeable / Status = readable consequence of stuff, would this be better named as VersionRoutingState?

hmm... its sort of in the grey area because "current_version" is an state from deployment POV but a version being current or not is a status from Version POV.

But now thinking more, maybe we don't need this enum. Because we'd have to have timestamps such as current_since and ramping_since for each version and if they have value it means the version is current or ramping.

I think there may be some controversy around this based on previous design discussions. So, let's not add it just yet. Instead the two timestamps of current_since and ramping_since should be enough for now. Once unversioned workers are supported we'd have a better idea about these.

sorry for the late review but how does the thought of encapsulating this under a message sound? Both these time stamps are dealing with routing information - looking below, the timestamps are under a single message

wdyt @carlydf ?

For the following reasons, I kinda prefer the previous format which had two separate timestamps better (but yeah, we should add ramp_percentage too):

• Based on @antlai-temporal 's feedback, we're going to support ramping version with 0% ramp. This format does not allow distinguishing that from "not ramping version".
• In the UI, we plan to tell user "ramping since x" where x is the time from start of the ramp, not the last ramp percentage change. (that info seems less valuable from user's POV)

so I guess what this would also mean is that we need server to have these separate timestamps present (maybe instead of routingUpdateTime) to keep things less-confusing

cc - @carlydf