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

Provisioning: Allow saving of provisioned dashboards #19820

Merged
merged 28 commits into from
Oct 31, 2019

Conversation

jongyllen
Copy link
Contributor

What this PR does / why we need it:
Allows saving of provisioned dashboards if the config value allowUiUpdates is set to true

This lets the user modify and save a dashboard that has been provisioned. It also enables some automation scenarios where the updated dashboards are pulled out via API calls and updated - provisioned to other instances etc.

Which issue(s) this PR fixes:
#11778
Allow UI to save changes for provisioned dashboards

Fixes #11778

Special notes for your reviewer:
Note that if updated on disk - the manually updated dashboard will be overwritten again by the provisioning engine. I recommend creating a separate Issue for this as it requires some thought around behaviour.

I would like the code to be a bit more readable an efficient but I could not find a way of doing that that would not require a big refactor of the services. If there are nice way of cleaning it up a bit, please give me some pointers!

@torkelo
Copy link
Member

torkelo commented Oct 18, 2019

This looks good.

Just needs some docs.

  • What happens when a provisioned dashboard updated in the UI, then later updated in json file
  • Is dashboard json version ever taken into account here (when json file is updated after UI save)?
  • What happens if json file or provisioned config is removed (and dashboard has an update form UI)?

@marefr marefr self-requested a review October 21, 2019 09:25
Copy link
Contributor

@marefr marefr left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Feels a bit weird to pass the allowUiUpdate flag to the DashboardService, but I understand why it is so and I don't think bringing in the ProvisioningService as a dependency to DashboardService is a good alternative. For now we have to live with the weirdness of the DashboardService that already existed before these changes.

I've added some docs suggestions. Still feel that the docs may need some more thoughts besides my suggestions. Maybe @marcusolsson can help out with this?

docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
Jon Gyllenswärd and others added 4 commits October 23, 2019 09:47
Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-Authored-By: Marcus Efraimsson <marcus.efraimsson@gmail.com>
@drewhemm
Copy link

This is a feature I am watching and eagerly anticipating. If I can be of any assistance in testing etc, please let me know.

I want to be able to hook into the save event somehow and automatically commit a change to the repo where we store our dashboard json.

@jongyllen jongyllen requested review from daniellee and marefr October 24, 2019 14:44
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

The scope of my review is only limited to the documentation. I'll leave the code part in the very capable hands of @marefr.

docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
docs/sources/administration/provisioning.md Outdated Show resolved Hide resolved
Copy link
Contributor

@marefr marefr left a comment

Choose a reason for hiding this comment

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

LGTM

@jongyllen
Copy link
Contributor Author

@drewhemm Sure! Feel free to test it and please give feedback if you have any. It will be in the next nightly build.

As for the second part - sounds like an interesting feature, feel free to raise an issue explaining in a little more detail what it should do and what kind of use-cases it would solve.

@jongyllen jongyllen merged commit a45ce36 into master Oct 31, 2019
@jongyllen jongyllen deleted the provisioned_dashboards_saving branch October 31, 2019 13:27
@jongyllen jongyllen added this to the 6.5 milestone Oct 31, 2019
@torkelo
Copy link
Member

torkelo commented Oct 31, 2019

Wohoo!

mixayloff-dimaaylov added a commit to mixayloff-dimaaylov/gstma that referenced this pull request Oct 22, 2022
В Grafana 6.5.1 появилась возможность сохранять изменения в *provisioned
dashboards*. Эти изменения не попадают в файловую систему, но это всё
равно удобнее, чем создавать клон дашборда.

Ref: grafana/grafana#19820
Docs: https://grafana.com/docs/grafana/v9.1/administration/provisioning/#dashboards
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.

Allow UI to save changes for provisioned dashboards
5 participants