Skip to content

The ability to customise request timeout in ConfigManager #326

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

Closed
GordonSo opened this issue May 7, 2021 · 1 comment
Closed

The ability to customise request timeout in ConfigManager #326

GordonSo opened this issue May 7, 2021 · 1 comment

Comments

@GordonSo
Copy link

GordonSo commented May 7, 2021

The default settings for

  • Blocking request timeout
  • Polling interval
  • Request timeout
    are all default to 10 seconds

Whilst the library offers the ability to override the Blocking and Polling settings, the request timeout is not exposed.
Can you confirm that this is a design decision, or would you be happy to accept our PR to expose the timeout setting also?

The reason this is important to us is because of the need to limit and have predictable message processing duration in our high-throughput system, and to de-risk ourselves from any network problem.

We would be happy to accept the risk that if we will fall back to the default variant on timeout, it can cause a discrepancy in the recorded metrics of the fallback variant being used instead of the variant that may be returned eventually; we expect this the occurrence should be rare.

@GordonSo
Copy link
Author

GordonSo commented May 12, 2021

Based on the helpful comment in PR #327 we learned that setting the "blocking_timeout" should be sufficient to serve our purpose.

The "request_timeout" is the polling thread timeout in the background which shouldn't be blocking.

nb: due to my limited understanding of the library, I'm just putting a timeout wrap via "Signal" on the Optimizely our function call for safe measures (see link). And I shall report back if the safe measure branch is ever required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant