Adds Global TTL Configuration for DynamoDB State Store#4055
Conversation
|
Would be nice when the different statestores (Redis and Dynamo) behave in a similar way. |
|
There will be a slight delay in reviewing the DynamoDB-related PRs. We need to address the missing/disabled tests first, which are being tracked in issue #4047. Thanks for your patience! |
|
@sicoyle Ready for another review round |
sicoyle
left a comment
There was a problem hiding this comment.
one last comment and then this LGTM - thank you!
Fixed. Also updated the docs PR with the same description |
b51bf00 to
e01dcdf
Compare
|
@ZeynelKoca pls see the build failures and ping when you're ready and I can retrigger 🙏 |
50d65a1 to
e01dcdf
Compare
|
@sicoyle Fixed a stupid rebase mistake. Try again? |
e8b1d9d to
8e8f446
Compare
8e8f446 to
494a6b9
Compare
|
@sicoyle Can the pipeline run get an approval? |
|
@sicoyle Fixed the formatting 🙏 |
|
@sicoyle I think a re-run is needed? Pipelines failed due to being unable to checkout the repo |
rerunning, lets see. I know github has been having quite a few issues recently |
Fixed another formatting issue 🙏 |
6b25c88 to
3eb43a9
Compare
3eb43a9 to
ced1c9e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mikeee Implemented your suggestion and 2 small (and valid) ones from copilot |
mikeee
left a comment
There was a problem hiding this comment.
lgtm, cheers for keeping on top of this.
|
/ok-to-test |
Complete Build MatrixThe build status is currently not updated here. Please visit the action run below directly. Commit ref: 07f3560 |
Components conformance testCommit ref: 07f3560 ❌ Some conformance tests failedThese tests failed:
Additionally, some tests did not report a status:
|
Components certification testCommit ref: 07f3560 ❌ Some certification tests failedThese tests failed:
Additionally, some tests did not report a status:
|
|
@mikeee can you rerun the workflows. Lots of tests failed due to failed secret setup |
Signed-off-by: Zeynel Koca <[email protected]>
07f3560 to
f53336d
Compare
Good catch, updated the cert test. TTL=-1 now means "no expiration" so the test asserts the item persists without a ttlExpireTime |
|
@mikeee we good to go here? |
|
/ok-to-test |
Complete Build MatrixThe build status is currently not updated here. Please visit the action run below directly. Commit ref: 3d4b873 |
Components certification testCommit ref: 3d4b873 ❌ Some certification tests failedThese tests failed:
Additionally, some tests did not report a status:
|
Components conformance testCommit ref: 3d4b873 ❌ Some conformance tests failedThese tests failed:
Additionally, some tests did not report a status:
|
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it was marked stale with no further activity for 7 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Description
Adds a way to globally configure the state TTL for dynamo (inspired by #1059).
The defined default TTL will only be used if the state request does not explicitly specify a TTL itself.
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: