spotify / luigi

Luigi is a Python module that helps you build complex pipelines of batch jobs. It handles dependency resolution, workflow management, visualization etc. It also comes with Hadoop support built in.
Apache License 2.0
17.71k stars 2.39k forks source link

Make Dropbox listdir optionally recursive #3260

Closed smrohrer closed 10 months ago

smrohrer commented 10 months ago

Description

Make the Dropbox listdir function accept an optional keyword argument recursive, True by default to preserve the current behavior.

Motivation and Context

Currently, listdir calls the Dropbox SDK files_list_folder function with recursive=True hard-coded. recursive should be optional, so we can replicate the functionality of LocalFileSystem.listdir which is not recursive.

Have you tested this? If so, how?

smrohrer commented 10 months ago

Thank you for the quick and diligent review.

I am closing this pull request because of a misunderstanding on my part: I confused the intended behavior of luigi.target.FileSystem.listdir with os.listdir - the former is meant to be recursive, and the latter is not.

However, I think the inclusion of the input path in the Dropbox listdir result does contradict the expected behavior of luigi.target.FileSystem.listdir. This is a behavior of the Dropbox files_list_folder function which is not described in the documentation.

If you think luigi.contrib.dropbox.DropboxClient.listdir should filter out the input path, I'd be happy to open another pull request.