Skip to content
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

feat(mads) allow specifying fetch-timeout via query param #2148

Merged
merged 8 commits into from
Jun 11, 2021

Conversation

austince
Copy link
Contributor

@austince austince commented Jun 10, 2021

Summary

Follow up on #2121, adds a query parameter (fetch-timeout) to allow clients to specify how long a single HTTP long polling request should wait.

Originally was thinking a GRPC TimeoutValue and TimeoutUnit made sense as the format, but we use golang's time.Duration format elsewhere in the config, and a parser already exists for it, so went with that instead.

If the timeout value is less than or equal to 0, the request does a sync fetch instead.

Full changelog

  • Add fetch-timeout query parameter to the MADS V1 HTTP endpoint
  • Also fixes a bug with the error responses continuing the request even after a response has been sent

Documentation

  • Will document after native Prometheus SD is in

Testing

  • Unit tests

austince added 3 commits June 10, 2021 14:00
Adds the `fetch-timeout` query parameter and parses
it according to the `time.Duration` spec.

Signed-off-by: austin ce <[email protected]>
Currently, when the HTTP service writes error
responses, the method does not return and continues
processing the erroneous request. This commit
fixes that.

Signed-off-by: austin ce <[email protected]>
@austince austince requested a review from a team as a code owner June 10, 2021 18:24
austince added 4 commits June 10, 2021 20:05
Signed-off-by: austin ce <[email protected]>
This reverts commit 957f712.

Signed-off-by: austin ce <[email protected]>
@austince austince merged commit 8bb2779 into master Jun 11, 2021
@austince austince deleted the feat/mads/long-polling-timeout branch June 11, 2021 02:26
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 this pull request may close these issues.

3 participants