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

feat(file-loader): Add recursive loading from sub-directories #5126

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

Conversation

EshaanAgg
Copy link

What type of PR is this?
Adds support for recursive file loading.

Which issue(s) this PR fixes:
Fixes #4689

Release Notes: Yes

new features: |
  Added support for recursive subdirectories in the FileProvider API

@EshaanAgg EshaanAgg requested a review from a team as a code owner January 22, 2025 14:41
Signed-off-by: EshaanAgg <[email protected]>
@EshaanAgg
Copy link
Author

@shawnh2
I have added the tests for some basic test cases in the case of nested directories in the new test suite. Since fsnotify does not support watching recursive directories, I listed all the sub-directories of the provided directories in paths, and then added a watcher on all of them.

I am currently a bit unsure about two things:

  • How should we be handling the various change events in this region of code:
    switch event.Op {
    case fsnotify.Create, fsnotify.Write, fsnotify.Remove:
    // Since we do not watch any events in the subdirectories, any events involving files
    // modifications in current directory will trigger the event handling.
    goto handle
    default:
    // do nothing
    continue
    }

    Leaving it as it allows the current test suite to pass, so I am not certain how we should handle it.
  • Should we add tests for cases where the user creates, deletes, or renames a complete directory in one of the watched directories, or is that unnecessary? Do we intend to support that functionality right now?

Sorry for the trouble associated with reviewing a new PR. And thanks! :)

Signed-off-by: EshaanAgg <[email protected]>
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

Successfully merging this pull request may close these issues.

Support recursive subdirectories for file-provider
1 participant