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

Required Version API Call #177

Open
safaci2000 opened this issue Oct 20, 2021 · 2 comments
Open

Required Version API Call #177

safaci2000 opened this issue Oct 20, 2021 · 2 comments

Comments

@safaci2000
Copy link
Contributor

This is more of a conversational starter than an issue, but I was wondering if there was a way to implement a behavior where we can tag the various API calls with a minimum/max version.

Aka we know that. client.GetHealth() requires grafana version X, so if grafana version is set either through a init method, or via an API call where state is populated, we would fail the SDK call more gracefully.

./bin/foobar server info
Error: /api/health requires Grafana version 1.2.3, you are connected to a server running version 0.1.2 which is lower than the minimum required version

Any thoughts?

@GiedriusS
Copy link
Collaborator

My initial idea was to split this SDK along major versions of Grafana. So, github.com/grafana-tools/sdk/v6 would only have code that works with the latest 6.x version; sdk/v7 only works with 7.x Grafana, and so on. I never got around to this due to lack of time because it's not so trivial - ideally, we'd share some code between all of those modules :/ I draw inspiration for this from: https://github.com/olivere/elastic#releases. What do you think about such idea?

@safaci2000
Copy link
Contributor Author

I'm not sure if I mentioned this, I'm working on a dashboard manager that uses this SDK to communicate with Grafana.

What I was thinking of doing is inspecting the server version and then based on the version fail a bit more gracefully.

A 404 or a stack trace isn't really that helpful over say unsupported version, you need to upgrade for feature X.

As long as you can import multiple versions of the API without conflicts that's fine. My concern is that for elastic there's usually not much of a use case where you're trying to support v6, v7 and v8 in the same code base.

There's two items that I find to be missing in the SDK.

  1. There are some API calls that require a basic auth admin, I haven't run into this but some likely only need an Org Admin, and some calls that don't support Token Admins (Which I have run into ). It would be nice to have some meta data associated with the calls to ensure the required auth and such is provided. This isn't critical but makes the code more elegant if you can say something like:
The Admin Organizations HTTP API does not currently work with an API Token. API Tokens are currently only linked to an organization and an organization role. They cannot be given the permission of server admin, only users can be given that permission. So in order to use these API calls you will have to use Basic Auth and the Grafana user must have the Grafana Admin permission (The default admin user is called admin and has permission to use this API).
  1. Is the Mixed case of versioning. Again both of these requests are mainly to allow for a more elegant program flow. a stack trace doesn't read as well as a "Sorry, version 2.2.2 is not supported."

I'm sure it's not very Go like, particularly since Go doesn't have annotations but my idea would be (My java background tainting me is showing up here)

@Version(min=6.0, max=8.0)
func GetServerInfoApi() {
//validateServerInfo
}

If we can try to have some patterns where we can try to ensure we can import multiple versions without too many issues, the /v6 /v7 IS a much cleaner implementation. Just voicing my concerns with that path.

Anyways, It would be REALLY nice to have an API definition from Grafana one of these days. It would make this whole versioning thing a bit easier to handle.

Anyways sorry for that rant, hopefully there's some value in this. Naturally i'm voicing my need specifically. It is a library so the common pattern should be given preference.

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

No branches or pull requests

2 participants