Fix _scandir bug #3
Conversation
trendelkampschroer
requested review from
birnbaum,
mathiaseitz,
MaxBenChrist and
ronaldrichter
| @@ -186,14 +186,12 @@ def _scandir(self, path: str, return_info: bool = False, namespaces: List[str] = | |||
| dir_key = self._path_to_dir_key(_path) | |||
There was a problem hiding this comment.
maybe we should look into the self._path_to_dir_key(_path) why it does not find the right dir_key and prefix. The change here feels kind of random (e.g. have to change dir_key = "/" but dfir_key = "/foo/ is fine
| prefix_len = len(prefix) | ||
| # In case we want to list the bucket, dir_key needs to be the empty string | ||
| dir_key = "" | ||
| prefix_len = len(dir_key) |
There was a problem hiding this comment.
I do not really understand what the problem was and why it is fixed now.
As far as I can see you only mutated dir_key instead of introducing a new variable prefix here (which I found to be more explicit, as the dir_key is actually correct, it just does not work with list_blobs).
There was a problem hiding this comment.
The problem was twofold:
i) prefix was overwritten by a loop variable created when iterating over prefixes
ii) some calls to list_blobs used dir_key instead of prefix (which was correct as long as dir_key != "/")
There was a problem hiding this comment.
I think it would be beneficial, if the behaviour of _path_to_key(root) and _path_to_dir_key(root) would be clearly defined if the root_path of the file system is the root of the bucket and root is any valid pyfilesystem specifier for the root of a filesystem.
|
This is obsolete due to #5 |
|
Closed by #5 |
This PR fixes a bug in the _scandir implementation. As a result of this bug _scandir did not list files at the root_path of the file-system, when the root_path of the file-system was identical to the top-level of the GCS bucket.