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

DataResponse: Add status code #544

Merged
merged 15 commits into from
Nov 1, 2022
Merged

DataResponse: Add status code #544

merged 15 commits into from
Nov 1, 2022

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Sep 29, 2022

In an effort to provide a more consistent error handling behavior, this PR adds a status code on the DataResponse object.

This is a simpler version of #510 -- rather than trying to enumerate the possible status codes developers may want to use, this just trusts they can find something appropriate in the standard HTTP status code list.

backend/json.go Outdated Show resolved Hide resolved
bytes jsonMeta = 3;

// Maps to raw HTTP status codes when passed over HTTP
int32 status = 4;
Copy link
Member Author

@ryantxu ryantxu Sep 29, 2022

Choose a reason for hiding this comment

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

adding line 148 is the one real protobuf change -- the rest are improved comments

@ryantxu ryantxu added the enhancement New feature or request label Sep 30, 2022
@ryantxu ryantxu marked this pull request as ready for review September 30, 2022 05:07
@ryantxu ryantxu requested a review from a team as a code owner September 30, 2022 05:07
@ryantxu ryantxu changed the title DataResponse: Add status and metadata to each response DataResponse: Add status code Oct 6, 2022
@wbrowne
Copy link
Member

wbrowne commented Oct 19, 2022

@ryantxu I have pushed a commit - LMK what you think 😄

sd2k added a commit to grafana/grafana-plugin-sdk-rust that referenced this pull request Oct 19, 2022
sd2k added a commit to grafana/grafana-plugin-sdk-rust that referenced this pull request Oct 19, 2022
backend/status.go Outdated Show resolved Hide resolved
backend/status.go Outdated Show resolved Hide resolved
wbrowne and others added 3 commits October 19, 2022 17:07
Co-authored-by: Ben Sully <ben.sully88@gmail.com>
@ryantxu
Copy link
Member Author

ryantxu commented Oct 25, 2022

This looks good to me -- @wbrowne / @marefr, if it seems good can you approve and merge?

We should also make sure the docs in #552 are updated to indicate what this status means

@wbrowne
Copy link
Member

wbrowne commented Oct 25, 2022

@ryantxu just going to do a once over of the tests and the marshalling logic

@wbrowne wbrowne merged commit 04d7c57 into main Nov 1, 2022
@wbrowne wbrowne deleted the response-with-status branch November 1, 2022 15:46
sd2k added a commit to grafana/grafana-plugin-sdk-rust that referenced this pull request Nov 4, 2022
sd2k added a commit to grafana/grafana-plugin-sdk-rust that referenced this pull request Jan 11, 2023
…or (#87)

* Add 'status' code to 'DataQueryError'; default to Internal Server Error

Example of how grafana/grafana-plugin-sdk-go#544 may
look in this SDK.

* Use custom DataQueryStatus enum instead of raw http status code

This narrows down the choice of options for users, while still ensuring
they choose a valid HTTP status code and allowing an escape hatch for
custom statuses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants