Allow query parameters on endpoints. Fix various endpoints.

grafana-toolbox / grafana-client

Python client library for accessing the Grafana HTTP API.

MIT License

106 stars 30 forks source link

Allow query parameters on endpoints. Fix various endpoints. #172

Closed chintal closed 7 months ago

chintal commented 7 months ago

Description

Fixes #171 Fixes #173 Fixes #174

Adds a util to format list-type params. Should not break any existing user code, but gives the option to provide such parameter values as a list instead of needing to construct a string of comma separated values.

Checklist

[ ] The patch has appropriate test coverage
[x] The patch follows the style guidelines of this project
[ ] The patch has appropriate comments, particularly in hard-to-understand areas
[ ] The documentation was updated corresponding to the patch
[x] I have performed a self-review of this patch

amotl commented 7 months ago

Dear @chintal,

thank you very much for your contribution. Your patch looks sane, I don't know why the PR fails on CI/GHA, we will need to look into that.

With kind regards, Andreas.

chintal commented 7 months ago

The failures look like they are some old tests failing. main by itself fails a number of tests. The PR is simple enough that it can be trivially rebased and applied once main is fixed.

chintal commented 7 months ago

My apologies for the combined PR for unrelated issues. I'll try to remember to create the PR from a branch next time.

chintal commented 7 months ago

By the way, the issue with the tests failing seems to be on line 163 of test_grafana_client.py and the behavior (?) of https://example.org.

https://github.com/panodata/grafana-client/blob/7f5712c7953bde359abcdbbfb783af29d0b59b0f/test/test_grafana_client.py#L163

The actual response received from example.com seems to be 500, though the text in the body of the response reads like a 404. So while the test checks for GrafanaClientError, it actually raises GrafanaServerError.

https://github.com/panodata/grafana-client/blob/7f5712c7953bde359abcdbbfb783af29d0b59b0f/grafana_client/client.py#L157-L172

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 76.66667% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 92.26%. Comparing base (7303ec2) to head (d341a2b). Report is 25 commits behind head on main.

Files	Patch %	Lines
grafana_client/elements/user.py	33.33%	4 Missing :warning:
grafana_client/elements/search.py	85.71%	2 Missing :warning:
grafana_client/util.py	75.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #172 +/- ## ========================================== - Coverage 94.44% 92.26% -2.19% ========================================== Files 26 27 +1 Lines 1711 1810 +99 ========================================== + Hits 1616 1670 +54 - Misses 95 140 +45 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

amotl commented 7 months ago

Dear Chintalagiri,

thank you so much for fixing those embarrassing flaws of grafana-client, and for adding new functionality around the User/Teams API endpoint wrapper. Excellent!

Keep up that good work, and with kind regards, Andreas.

amotl commented 7 months ago

grafana-client 4.1.0 has been released, including your fixes and improvements. Thanks again.

NB: Fixing those flaws so diligently and swiftly clearly makes you eligible to be added as a collaborator on this project, so we've just invited you. Feel free to join if that fits your bill.