-
Notifications
You must be signed in to change notification settings - Fork 68
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
Update core #660
base: main
Are you sure you want to change the base?
Update core #660
Conversation
May have to upgrade tonic and tonic build (probably just a |
@dandavison - I am a little concerned here with temporalio/sdk-core#789 and initialize workflow. It appears that it's more than a name change, it is also an order change. From the PR description:
I think we are saved by the fact that we reorder events ourselves in Python. I can't think of any non-signal/non-update/non-update-random-seed event that ever came before start in Core before that PR was created, so I think we're ok. But may be worth making sure we take a close look and confirm. |
I agree. I think this PR should be the first PR post-release, rather than the last PR pre-release. |
👍 It would have been nice to get temporalio/sdk-core#818 and temporalio/sdk-core#815 and some others in this release, but I understand that reviewing this core change may take longer than we're wanting for some pure Python changes pending release. |
My intention was that this upgrade should also include the changes to avoid applying jobs in different sets and thus match the behavior mentioned in #606 Having all the SDKs work the same way in that regard will be a long term win I think, and is in general just a bit simpler to think about. |
(added #671 as being closed by this) |
Update core to temporalio/sdk-core@8691fed