-
Notifications
You must be signed in to change notification settings - Fork 33
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
Refine API wrapper for "Plugins" once more #115
Conversation
Codecov Report
@@ Coverage Diff @@
## main #115 +/- ##
==========================================
- Coverage 94.85% 94.11% -0.75%
==========================================
Files 25 25
Lines 1594 1613 +19
==========================================
+ Hits 1512 1518 +6
- Misses 82 95 +13
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
a63bc94
to
bbb2ef6
Compare
bbb2ef6
to
402cb5e
Compare
""" | ||
:return: | ||
Return list of all installed plugins. | ||
""" | ||
path = "/plugins?embedded=0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we allow user to pass the filters like embedded=0&core=0
? Can be a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, why not. Can you point us to corresponding documentation or code, where all the possible filtering parameters are enumerated? Maybe there are even more, about sorting or paging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, see GH-117.
path = "/plugins/%s/install" % plugin_id | ||
# Unfortunately, this endpoint may respond with an empty JSON, | ||
# which needs compensation, because it does not decode well. | ||
r = self.client.POST(path, json={"version": version}, accept_empty_json=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for handling this , I was not sure on how to handle the exceptions I was getting.
return r | ||
except GrafanaClientError as ex: | ||
# Ignore `Client Error 409: Plugin already installed`. | ||
if "409" not in str(ex): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, I figured after I merged my PR.
About
After GH-110 and GH-114, this patch streamlines the API wrapper about "plugins" a bit more.
Details
The patch is needed to make Add subcommands
plugins {list,status}
, to inquire plugins grafana-wtf#91 work.While at it, it also adjusts/trims the method names a bit. I hope it will not cause too much harm, because the feature has only been released recently. I don't think we need to bump to 4.0.0 to properly signal that "breaking" change. I would just run a 3.9.0 release to ship it. Please let me know if you think differently.
/cc @bhks