experimental: adjust the query client interface, remove unused code #1211
Conversation
| func NewErrorQDR(req QueryDataRequest, err error) *backend.QueryDataResponse { | ||
| qdr := backend.NewQueryDataResponse() | ||
| for _, q := range req.Queries { | ||
| qdr.Responses[q.RefID] = backend.ErrDataResponse(backend.StatusBadRequest, err.Error()) |
There was a problem hiding this comment.
I think it's weird that the existing code did this btw... like what if the status code from the response was a 429 it would get lost kind of? But also my understanding is maybe that's already happening so i guess ok to keep it? haha.
There was a problem hiding this comment.
yeah, i admit, we kind of rely on knowing (or thinking that we know 😁 ) the possible http-codes.
i did some digging for how /api/ds/query does this, and it's like this:
- https://github.com/grafana/grafana/blob/58df80e542201b91e480c3425ecce6dcd792c1d3/pkg/api/ds_query.go#L80
- this seems to be the point where QueryDataResponse (QDR) structures become http-status-codes and json... as you can see, it checks if we have an
erroror a QDR- if we have an
error, then it goes to https://github.com/grafana/grafana/blob/58df80e542201b91e480c3425ecce6dcd792c1d3/pkg/api/ds_query.go#L24, and , interestingly, checks for the error being a http404-error, and handles that specially. the rest seems to be auth-related. and if nothing matches, http500. - if we don't have an
error, meaning we have aQDR, then we go to https://github.com/grafana/grafana/blob/58df80e542201b91e480c3425ecce6dcd792c1d3/pkg/api/ds_query.go#L86, and we check if the QDR has any DataResponses in it with an error. if yes, http400, else http200.
- if we have an
- this seems to be the point where QueryDataResponse (QDR) structures become http-status-codes and json... as you can see, it checks if we have an
so in short, if we have a QDR, then it's http400 or http200, if we don't have a QDR, we check for maybe-http404, some auth-cases, else http500 😄
... of course it's still possible that we'll find a scenario that we cannot represent with the current approach 😿 , i guess then we go back to the drawing board again..
gabor
force-pushed
the
gabor/query-client-change
branch
from
50b1c76 to
84e1084
Compare
gabor
force-pushed
the
gabor/query-client-change
branch
from
84e1084 to
aef7827
Compare
gabor
requested review from
wbrowne,
andresmgot and
oshirohugo
and removed request for
a team
|
will use a different approach, closing for now |
changes to certain structures in
experimental:QueryDataClientto be a better fit for it's consumers. (example consumer: datasources: querier: adjust the query-client grafana#99805)simpleHTTPClientimplementation, and it's dependencies, as it is not used anymore.