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

feature: ListFolders: folders with invalid name should just be dropped, not cause a fatal error #322

Open
aerickson opened this issue Dec 9, 2024 · 1 comment
Assignees
Milestone

Comments

@aerickson
Copy link

Is your feature request related to a problem? Please describe.
Feature. Invalid folders shouldn't be a fatal error, they should just be ignored.

Describe the solution you'd like
Invalid folders are ignored, and the program can continue.

Describe alternatives you've considered
I could have nagged the folder owner to change the folder's title.

Additional context
This diff allowed me to backup a valid folder (despite the invalid folder existing).

diff --git a/internal/service/folders.go b/internal/service/folders.go
index b8421cf..3f63435 100644
--- a/internal/service/folders.go
+++ b/internal/service/folders.go
@@ -206,7 +206,7 @@ func (s *DashNGoImpl) ListFolders(filter filters.Filter) []*types.FolderDetails
        for ndx, val := range folderListing {
                valid := s.checkFolderName(val.Title)
                if !valid {
-                       log.Fatalf("Folder has an invalid character and is not supported. Path separators are not allowed. folderName: %s", val.Title)
+                       log.Printf("Folder has an invalid character and is not supported. Path separators are not allowed. folderName: %s", val.Title)
                }
                filterValue := val.Title
                var nestedVal string
@safaci2000
Copy link
Contributor

Hmm, this has other implications. The last version added support for nested folders which has become a standard feature available in grafana now. So Ignoring the folder also affects all potential dashboards within that folder. It also impacts the dashboards.

The issue with the 'warning', is that there's folks that run GDG in a crontab or as kubenetes job, in which case nobody is really actively reading the logs as it's just expected to work. When an operation is potentially dropping an entire folder and all its content from being backed up. I think the fatal error is correct.

We could potentially introduce a configuration option to allow this but I definitely don't think it should be the default behavior. The special chars that are checked for intentionally because they have an impact on other features of GDG besides just the folder names.

@safaci2000 safaci2000 added this to the 0.7.2 milestone Dec 18, 2024
@safaci2000 safaci2000 self-assigned this Dec 18, 2024
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

No branches or pull requests

2 participants