-
Notifications
You must be signed in to change notification settings - Fork 102
Versioning Override support #871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
04f474e
to
3eec5c5
Compare
temporalio/common.py
Outdated
@@ -1037,6 +1039,17 @@ class VersioningBehavior(IntEnum): | |||
queue) when the next task is dispatched.""" | |||
|
|||
|
|||
class PinnedOverrideBehavior(IntEnum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider calling this PinnedVersioningOverrideBehavior
since it's related to versioning but in the common area
temporalio/common.py
Outdated
@@ -1037,6 +1039,17 @@ class VersioningBehavior(IntEnum): | |||
queue) when the next task is dispatched.""" | |||
|
|||
|
|||
class PinnedOverrideBehavior(IntEnum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of the single-value enum here? Can we just leave this code off until there is a second value that can ever be set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More are definitely coming, but sure can leave it off for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of comments, but nothing blocking
) | ||
|
||
|
||
class VersioningOverride(ABC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this may be set on child workflow or activity invocations one day and is why it is in common
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes (or available on context info)
) | ||
|
||
|
||
class AutoUpgradeVersioningOverride(VersioningOverride): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still probably worth a frozen dataclass here so you get the other goodies (hash, str, etc)
…n into versioning-override
What was changed
Add versioning override support to start workflow calls
Why?
Parity
Checklist
Closes
How was this tested:
Added test
Any docs updates needed?