Provisioning: Remove provisioned dashboards without parental reader #26143
Conversation
nabokihms
force-pushed
the
delete-orphaned-provisioned-dashboards
branch
from
bc66b95 to
26cf253
Compare
|
Nice! :) 🎆 I won't have time to review this as I'll be on vacation for a while but I'll make sure to help someone catching up on provisioning to be able to review this. |
kylebrandt
requested review from
a team,
aknuds1 and
marefr
and removed request for
a team and
kylebrandt
|
Do you mind merging this with master, in order to include Drone CI configuration? |
nabokihms
force-pushed
the
delete-orphaned-provisioned-dashboards
branch
from
edacd35 to
5953e0f
Compare
|
@aknuds1 I applied your suggestions, moved commentary, and rebased the current branch on the master. The drone pipeline passed. |
|
Thanks @nabokihms. |
There was a problem hiding this comment.
Looks good AFAICT, but I'll let @bergquist have his say since I'm new to the functionality.
| } | ||
|
|
||
| for _, deleteDashCommand := range result { | ||
| err := deleteDashboard(&models.DeleteDashboardCommand{Id: deleteDashCommand.DashboardId}, sess) |
There was a problem hiding this comment.
Would be great if we could move this deletion loop outside the inTransaction wrapper. This could create very large transactions without any clear upsides. I suggest we query for potential dashboards to delete without a transaction and then delete each dashboard in its own transition.
There was a problem hiding this comment.
Yes, it makes sense. I have changed the DeleteOrphanedProvisionedDashboards function behavior according to your suggestions. To make CI works, I also rebased the branch onto master.
nabokihms
force-pushed
the
delete-orphaned-provisioned-dashboards
branch
from
074bb99 to
821a636
Compare
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
nabokihms
force-pushed
the
delete-orphaned-provisioned-dashboards
branch
from
821a636 to
a2cf1fa
Compare
Signed-off-by: m.nabokikh maksim.nabokikh@flant.com
What this PR does / why we need it:
This PR adds deletion of provisioned dashboards, which have no link with provision reader configuration (or you can say, that they are orphan).
The deletion process starts on the Grafana load and occurs on every dashboard provision configuration reloading (requesting
/api/admin/provisioning/dashboards/reloadendpoint).Which issue(s) this PR fixes:
Fixes #15716
Special notes for your reviewer:
There are two problems:
providers[].disableDeletionflag, because there is no information about previous file readers' settings in the database. In my opinion, to solve this problem we need to add a column to thedashboard_provisioningtable and store the information about dashboards deletion possibility in it.I also wanted to mention that I tried to reuse existed code as much as possible.