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

change query signature #2

Merged
merged 2 commits into from
May 6, 2020
Merged

change query signature #2

merged 2 commits into from
May 6, 2020

Conversation

kylebrandt
Copy link
Contributor

I don't think returning an error from query is a good pattern (although, one might return an error from functions within it, and then assign to response.Error).

Could restore comment about what happens when returning an error to the handle in calling function. Don't care about the named return if you don't want to use it here, but I think cleaner when there is no initialization to do and a single return param.

pkg/datasource/sample-datasource.go Outdated Show resolved Hide resolved
@@ -33,10 +33,7 @@ func (td *SampleDatasource) QueryData(ctx context.Context, req *backend.QueryDat

// loop over queries and execute them individually.
for _, q := range req.Queries {
res, err := td.query(ctx, q)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted to show that you can err in different ways. But that might not be suitable for tutorials/boilerplate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, could move it into QueryData as per initial comment on PR.

@kylebrandt kylebrandt merged commit 31084fc into master May 6, 2020
@kylebrandt kylebrandt deleted the changeError branch May 6, 2020 13:34
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.

None yet

2 participants