esnet / gdg

Grafana Dashboard Manager
https://software.es.net/gdg/
Other
344 stars 34 forks source link

support nested dashboard-folders for backup-download #287

Closed Iridias closed 1 month ago

Iridias commented 2 months ago

Our use-case: We use gdg only as a backup-tool (no restore).

The issue: A while ago, Grafana introduced the feature of nested sub-folders for dashboards. So far gdg would download those folders, but all on the same level. This has (at least) two major problems:

What changed:

Known Issues:

safaci2000 commented 2 months ago

First of all, thank you for the PR, that's always appreciated. I have looked at nested folder when they first were released as a beta feature and it felt very broken still at the time, and ended up closing that out till someone asked for the feature. (Which I suppose you're asking.)

A few questions.

  1. I know this is a toggle, but this is now an officially supported feature right? out of beta?
  2. I'm happy to try to add support for nested folders but I'd like to do this fully, ie. GDG should have a setting to use/not use internal folders, and have the ability to download, upload, list and restore doing the full CRUD operations.

Let me take a closer look at the MR, and see how it works a bit closer. I'll try to suggest something that should work for all use cases.

safaci2000 commented 2 months ago

Please link: https://github.com/esnet/gdg/issues/171 to your PR as well.

safaci2000 commented 2 months ago

There's a few things I'd like to add to this, I'm going to pick up on your changes and take it the last hurdle so it lines up a bit better. I also wanted to get integration tests written for the cloud and basic CRUD that uses nested folders. Once I have this cleaned up, I'll ping you on a final pass if you wanted to double check it satisfies your use case.

From what I can tell you mainly wanted the ability to fetch nested folders and respect those deeper paths.

Iridias commented 1 month ago

Sorry for the late response - was on vacation.

I'm going to pick up on your changes and take it the last hurdle so it lines up a bit better.

Cool. Thanks for the effort :-)

From what I can tell you mainly wanted the ability to fetch nested folders and respect those deeper paths.

Yes, that's right.

safaci2000 commented 1 month ago

@Iridias somewhat. Once you enable that flag it affects the inherent behavior of Dashboard CRUD, Folder CRUD and Folder Permissions as well. So it's a bigger change to introduce it. It's still a WIP but I have the rough Impl done just need some clean up and testing and likely removing some duplicate logic that can be consolidated.

safaci2000 commented 1 month ago

@Iridias I'm going to close this patch. Please see https://github.com/esnet/gdg/pull/291 for this feature change.