Add FirstRunAt field to start workflow option logic #6178
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…WorkflowExecutionRequest, and StartChildWorkflowExecutionInitiatedEventAttributes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
... and 23 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
timl3136
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
sankari165,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
timl3136
changed the title
|
Can you update PR description with testing details? Please cover how you tested with old and new clients. |
taylanisikdemir
approved these changes
Jul 26, 2024
Groxx
added a commit
that referenced
this pull request
Aug 20, 2024
…th go module (#6241) `idls/` and the `github.com/uber/cadence-idl` version in `go.mod` are currently out of sync and more than a little screwed up: - the go module is on `0482c040f91d17be35cccee2bc1cf883f7344992` - the IDL submodule is on `1cd936eb8e24d42f8d8cd38d6216e619c14c48d5` The go module is currently on cadence-workflow/cadence-idl@0482c04 which is the latest `master` and seems fine. The current IDL submodule: https://github.com/uber/cadence-idl/tree/1cd936eb8e24d42f8d8cd38d6216e619c14c48d5 doesn't belong to any branch (much less `master`), and is in danger of being GC'd if we don't do something about that. It's probably from a temporary PR branch that was cleaned up as part of #6178 (the contents are similar to https://github.com/uber/cadence-idl/tree/d92bb53292d64b698c0c5600bbd6e258e728a020 which was approved and _is_ on `master`), but I can't find `1cd936eb8e24d42f8d8cd38d6216e619c14c48d5` in any PR in either https://github.com/uber/cadence-idl or https://github.com/timl3136/cadence-idl so I'm really not sure. So this PR contains a few changes: - moves `idls/` to match `go.mod` SHA: `0482c040f91d17be35cccee2bc1cf883f7344992` - `d92bb53292d64b698c0c5600bbd6e258e728a020` did not build, as the service code for `ListAll...` was removed, which is also why I didn't just revert the related commits. - adds a `make .idl-status` and CI steps to verify that this all stays in sync in the future, because this is all VERY easy to miss in PR reviews, and github's UI just makes it worse by hiding many of these details.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed?
Add FirstRunAt field logic handling to start workflow option logic. The field will override delay start and jitter start seconds for the first run of the workflow in order to provide a precise start time.
Why?
We need a mechanism to allow the first run of a workflow to be schedule at some exact time. Currently, the only way is to calculate the delay start required and that is often not accurate enough. If the user want to modify the first run of a cron workflow, they need to know the precise timestamp when they run the command in order to calculate the necessary delay start seconds needed.
How did you test it?
Unit tests, also tested in local as well as dev environment.
For compatibility testing, we tested the change in following configuration ensuring that it maintain compatibility:
Server w/ change with client w/o change
Server w/o change with client w/ change
Server w/ change with client w/ change.
Potential risks
There might be incompatibility issues between different versions of server and client. But in the worst case, the logic will be ignored and not alter existing logic in other ways.
Release notes
Documentation Changes