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

Better idempotence report when creating a dashboard in a folder #168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BenjaminSchubert
Copy link
Contributor

SUMMARY

Previously, whenever you would upload a dashboard to a custom folder, the status would always be reported as changed, as the folderId is never included in the return value, but was in our payload.

The current solution is not perfect, but would at least not report changed when nothing changed, and should not break any other workflows.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

grafana_dashboard

ADDITIONAL INFORMATION

In order to understand and reproduce the issue, running the added integration test without the patch would fail, as the second upload would fail.

For context, grafana/grafana#12491 is also relevant

This generates smaller diff if we want to modify it and run various
tests with variants
This enables us to stop having a task reported as changed when
uploading a dashboard repeatedly in a folder that is not the main
folder.

The grafana API doesn't return the 'folderId', and thus we would
previously always be reported as 'changed'.
@codecov
Copy link

codecov bot commented Apr 25, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@e015c80). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #168   +/-   ##
=======================================
  Coverage        ?   72.50%           
=======================================
  Files           ?        7           
  Lines           ?      702           
  Branches        ?       75           
=======================================
  Hits            ?      509           
  Misses          ?      176           
  Partials        ?       17           

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 e015c80...61a4dbc. Read the comment docs.

@BenjaminSchubert
Copy link
Contributor Author

Another solution would be first ot get the dashboard by uid then search for it by title, which would give a more accurate result, for example, if the person is renaming the dashboard. I am happy to change my solution to such a system if you prefer.

@rrey
Copy link
Collaborator

rrey commented Sep 8, 2021

Hi @BenjaminSchubert !
Thanks for your PR !

I think that first looking for the dashboard by uuid and then by title would indeed avoid having a match by title while we have an uuid match later on.

I gladly accept the improvement !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants