Skip to content

Include parameter name when returning HTTP 400 for query_range API #3703

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

Conversation

alvinlin123
Copy link
Contributor

@alvinlin123 alvinlin123 commented Jan 17, 2021

What this PR does:
Add parameter name to HTTP 400 message returned to the user for self-debugging.

Which issue(s) this PR fixes:
Fixes #3699

Checklist

  • Tests updated
  • [] Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Alvin Lin <alvinlin@amazon.com>
@alvinlin123 alvinlin123 force-pushed the issue-3699-better-error-message-for-query-range branch 2 times, most recently from 0c3ca56 to 8cfe522 Compare January 17, 2021 10:24
Signed-off-by: Alvin Lin <alvinlin@amazon.com>
@alvinlin123 alvinlin123 force-pushed the issue-3699-better-error-message-for-query-range branch from 8cfe522 to daaf429 Compare January 17, 2021 10:26
Signed-off-by: Alvin Lin <alvinlin@amazon.com>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (with Marco's comments).

(Personally I find util.ParseTime returning httpgrpc.Error directly ugly, leading to more ugliness in decorateWithParamName, but there is no need to spend more time on it.)

@pracucci pracucci merged commit b1be491 into cortexproject:master Jan 19, 2021
simonswine pushed a commit to simonswine/cortex that referenced this pull request Jan 19, 2021
…ortexproject#3703)

* Include parameter name when returning HTTP 400 for query_range API

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Add param name even if error is not rpc error

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Fix typo in CHANGELOG.md

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update CHANGELOG.md

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Co-authored-by: Marco Pracucci <marco@pracucci.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error message when parameter is missing when calling query_range API
3 participants