Remove apiVersion from root PluginContext #1015
Conversation
andresmgot
requested review from
wbrowne,
marefr,
oshirohugo and
ryantxu
and removed request for
a team
| @@ -81,9 +81,6 @@ message PluginContext { | |||
|
|
|||
| // The user agent of the Grafana server that initiated the gRPC request. | |||
| string userAgent = 8; | |||
|
|
|||
| // The API version that initiated a request | |||
| string apiVersion = 9; | |||
There was a problem hiding this comment.
Sure this is fine even though not in use? Have you tested it? Like if a new field 9 is added later with a different type and there are grafana/plugin versions still depending on the old version etc. You never delete anything is what I've learned. I think there's a trick to keep the index unnamed or something
| @@ -81,9 +81,6 @@ message PluginContext { | |||
|
|
|||
| // The user agent of the Grafana server that initiated the gRPC request. | |||
| string userAgent = 8; | |||
|
|
|||
| // The API version that initiated a request | |||
| string apiVersion = 9; | |||
There was a problem hiding this comment.
I don't understand why this should be removed. Any requests starting from apiservers should include it. This is how the backend can consistently know the apiversion for all grpc requests
There was a problem hiding this comment.
This is a bit fuzzy to me, the apiservers don't start requests, do they? Meaning, the request comes from either a frontend, the alert runner or similar. Why would it append an api version to a request it's receiving?
There was a problem hiding this comment.
Comment made offline by @marefr:
Okay. But datasource api server receiving a request will have the api version in the URL and to route that to backend plugin handling that specific api version you might need to forward the api version 🤷 In that case, plugin context seems like a good fit. That's my thinking
Which makes sense. Thanks both for the input!
We are not using this atm but yeah, we may want to use it. I'll close this PR 👍
What this PR does / why we need it:
Continue clean up from grafana/grafana#89830. The apiVersion is set as part of the DatasourceInstanceSettings so it does not need / should not be directly in the PluginContext struct. This was removed in grafana/grafana here: https://github.com/grafana/grafana/pull/89831/files#diff-663e8e1a8cdfbbf4d139f9d2cdd32681687fbe625e79d02d08e69a03519c71f5L46
This is marked as a breaking change (both for removing the field from the proto and the struct) but since it was not being used it should be fine. Let me know if you disagree.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: