fs: temporarily disable prefix-based search for fsspec-implementations #6096
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pmrowla
approved these changes
Jun 2, 2021
efiop
reviewed
Jun 2, 2021
Comment on lines
+44
to
+51
| # Clouds that implement the general methods that can be tested | ||
| # for functional tests that require extensive apis (e.g traversing | ||
| # via walk_files()) | ||
| full_clouds = [ | ||
| pytest.lazy_fixture(cloud) | ||
| for cloud in ["s3", "gs", "azure", "ssh", "hdfs"] | ||
| ] | ||
|
|
There was a problem hiding this comment.
Let's add these directly to test_pull_00_prefix parametrization, since they are not used anywhere else.
efiop
reviewed
Jun 2, 2021
Comment on lines
+402
to
+406
| # During the tests, for ensuring that the traverse behavior | ||
| # is working we turn on this option. It will ensure the | ||
| # list_hashes_traverse() is called. | ||
| always_traverse = getattr(self.fs, "_ALWAYS_TRAVERSE", False) | ||
|
|
There was a problem hiding this comment.
It feels a bit weird to amend the core code like that only for the tests. I guess this is a hint for an upcoming refactor of this part of code 🙁
There was a problem hiding this comment.
@pmrowla Do you foresee this part being refactored to accommodate the test better? Maybe you have any suggestions?
|
Ok, let's merge and release, but discuss ^^^ Thank you! 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a temporary fix for #6089. It disables the partial prefix (
00/0) search optimization entirely for all fsspec implementations and does it in the regular way (with only listing the full path of00/). This API is not actually supported to be optimized on the fsspec side so we will wait for all the clouds we support (s3, azure, google cloud) and then turn on the optimization (In theObjectFSWrapper) this time with just fixing the customfind()method in the subclass.