feat(openfeature): decrease default initialization timeout to 10s#18169
Conversation
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 6df213e | Docs | Datadog PR Page | Give us feedback! |
dd-oleksii
added
the
changelog/no-changelog
|
Setting changelog/no-changelog to silence the warning — I modified the initial release note for the feature. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1c7f7f1a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
We found that 30s timeout doesn't play well with gunicorn, which has 30s initialization timout for the worker. If the user naively calls `openfeature.api.set_provider()` at the top-level, RC failing to deliver configuration may bring their whole application down with hard-to-debug `[CRITICAL] WORKER TIMEOUT` error message. Decrease default timeout to 10s, which is enough when RC is working but not too long to cause other issues/upstream timeouts.
dd-oleksii
force-pushed
the
oleksii/jj-pwzqvtvtzmsy
branch
from
f1c7f7f to
6df213e
Compare
Codeowners resolved as |
|
PR #18160 Is going to disable |
|
@brettlangdon do you mean #18163? Yes, we need to re-enable that scenario after this is merged |
|
@dd-oleksii both PRs are doing the same thing, we'll close #18163 in favor of #18160 |
cbeauchesne
left a comment
cbeauchesne
left a comment
There was a problem hiding this comment.
Could your remove FEATURE_FLAGGING_AND_EXPERIMENTATION from excluded_scenarios here ? https://github.com/DataDog/dd-trace-py/blob/main/.github/workflows/system-tests.yml#L147C27-L147C63
|
@cbeauchesne this PR was open and tests ran before that disable landed. #18172 is set up ready to restore the coverage |
|
This change is marked for backport to 4.9, but it conflicts with that branch. The command used to test backporting was |
brettlangdon
dismissed
cbeauchesne’s stale review
Follow-up is scheduled to add back system-tests coverage, we'll get this PR merged and backported to 4.9 quickly to get it into 4.9.0
|
This change is marked for backport to 4.10, but it conflicts with that branch. The command used to test backporting was |
gh-worker-dd-mergequeue-cf854d
Bot
merged commit 017bc2a
into
main
…8169) <!-- Provide an overview of the change and motivation for the change --> We found that 30s timeout doesn't play well with gunicorn, which has 30s initialization timout for the worker. If the user naively calls `openfeature.api.set_provider()` at the top-level, RC failing to deliver configuration may bring their whole application down with hard-to-debug `[CRITICAL] WORKER TIMEOUT` error message. Decrease default timeout to 10s, which is enough when RC is working but not too long to cause other issues/upstream timeouts. <!-- Describe your testing strategy or note what tests are included --> <!-- Note any risks associated with this change, or "None" if no risks --> <!-- Any other information that would be helpful for reviewers --> Part of incident-54756, follow up to #16650 Co-authored-by: brett.langdon <brett.langdon@datadoghq.com> (cherry picked from commit 017bc2a)
…8169) ## Description <!-- Provide an overview of the change and motivation for the change --> We found that 30s timeout doesn't play well with gunicorn, which has 30s initialization timout for the worker. If the user naively calls `openfeature.api.set_provider()` at the top-level, RC failing to deliver configuration may bring their whole application down with hard-to-debug `[CRITICAL] WORKER TIMEOUT` error message. Decrease default timeout to 10s, which is enough when RC is working but not too long to cause other issues/upstream timeouts. ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers --> Part of incident-54756, follow up to #16650 Co-authored-by: brett.langdon <brett.langdon@datadoghq.com>
4 participants
Description
We found that 30s timeout doesn't play well with gunicorn, which has 30s initialization timout for the worker.
If the user naively calls
openfeature.api.set_provider()at the top-level, RC failing to deliver configuration may bring their whole application down with hard-to-debug[CRITICAL] WORKER TIMEOUTerror message.Decrease default timeout to 10s, which is enough when RC is working but not too long to cause other issues/upstream timeouts.
Testing
Risks
Additional Notes
Part of incident-54756, follow up to #16650