Skip to content

Conversation

@peekjef72
Copy link
Contributor

@peekjef72 peekjef72 commented Feb 6, 2022

Description

  • Add interface to Grafana's "health" endpoint (/api/health)
  • Add new method get_datasource_proxy_data() (/api/datasource/proxy...) on "datasource" element to be able to collect data info through Grafana.

Notes

Checklist

  • The patch has appropriate test coverage
  • 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
  • I have performed a self-review of this patch

@peekjef72 peekjef72 requested a review from amotl as a code owner February 6, 2022 10:20
@peekjef72
Copy link
Contributor Author

Hi Andreas,

It is a very good news, that you decided to fork the original project, if it helps to develop it !
Thanks.

If there are problems with this PR, just tell me, particullarly concerning code coverage (i'm really not famillar with the concept).

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2022

Codecov Report

Merging #5 (c4f7a62) into main (73d51c1) will increase coverage by 1.60%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   85.87%   87.48%   +1.60%     
==========================================
  Files          16       17       +1     
  Lines         637      655      +18     
==========================================
+ Hits          547      573      +26     
+ Misses         90       82       -8     
Impacted Files Coverage Δ
grafana_client/elements/datasource.py 60.41% <85.71%> (+26.27%) ⬆️
grafana_client/api.py 100.00% <100.00%> (ø)
grafana_client/elements/__init__.py 100.00% <100.00%> (ø)
grafana_client/elements/health.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36d0a50...c4f7a62. Read the comment docs.

Copy link
Contributor

@amotl amotl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @peekjef72,

thank you very much for your contribution. I added a minor comment below, where things might get improved. Please let me know if you think I am getting things wrong.

With kind regards,
Andreas.

r = self.client.DELETE(delete_datasource)
return r

def get_datasource_proxy_data(self, datasource_id
Copy link
Contributor

@amotl amotl Feb 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are problems with this PR, just tell me. Particularly concerning code coverage, because I'm not really familiar with this concept.

I can't see where get_datasource_proxy_data() will be called from any code, including test cases. This is probably why Codecov reported about a 0.52% decline in code coverage at #5 (comment).

In order to learn something about the general concept, I would like to refer you to 1. Maybe you can add a corresponding test case for this function? Please let me know if you need further assistance or clarification on this detail.

Footnotes

  1. https://en.wikipedia.org/wiki/Code_coverage

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the documentation section 1 does not contain any example about how to use the datasource element. Feel free to add a short example there about its baseline usage, and in particular about its usage with get_datasource_proxy_data. Do you think this is a good idea?

Footnotes

  1. https://github.com/panodata/grafana-client#getting-started

@peekjef72
Copy link
Contributor Author

peekjef72 commented Feb 6, 2022

I've added tests for the datasource part (find, get, get_dtasource_proxy_data).
The "get_datasource_proxy_data" is used to retrieve data from a grafana backend datasources.
I use them in a small tool grafana-snapshots-tool that use the previous version grafana_api, and allow to generate dashboards containing data (so snapshots !).
I hope the code is ok now.

@amotl
Copy link
Contributor

amotl commented Feb 6, 2022

Dear @peekjef72,

your grafana-snapshots-tool looks great, thanks for sharing!

Also, the test cases you added look pretty comprehensive, increasing the test coverage by 1.60 %, see #5 (comment). Thanks a stack!

With kind regards,
Andreas.

P.S.: I would like to add m0nhawk/grafana_api#85 before cutting a new release to be published on PyPI. I hope you are fine with that.

@amotl
Copy link
Contributor

amotl commented Feb 6, 2022

Hi again,

your improvements became part of grafana-client-2.1.0, which was just released. Thanks again!

With kind regards,
Andreas.

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

Successfully merging this pull request may close these issues.

3 participants