refactor(fsspec): remove upper bound on fsspec #221
Conversation
We're using `pins` as a dependency in `ibis-framework` and ideally we can remove the upper-pin on `fsspec` so that doesn't impact our users. I think the usage of the moved `hash_name` function is the only breaking change between the current upper pin and most recent release, so I've added a small workaround to handle the change. All of the local tests pass, I didn't yet set up sufficient credentials to handle the various cloud tests.
gforsyth
force-pushed
the
remove_fsspec_bound
branch
from
80a2784 to
a210de5
Compare
|
Hi there @gforsyth -- thank you so much for the contribution! This will be great for ibis and also pins to be able to support all fsspec versions again 🤝 I just reran the CI with all the relevant secrets and am seeing some failures that, at first glance, seem unrelated to your changes. Let me do some spelunking this afternoon, then hopefully we can merge this in! |
|
Thanks for taking a look, @isabelizimm ! |
|
Are you able to check what version of fsspec you are testing on? Locally I'm able to hit these same errors when upgrading to the latest version, I'm happy to build off your branch and fix those pieces, if you are okay with that! |
|
cc: @machow have you ever seen the I'm not wholly opposed to bumping the depth, but would like to avoid it. |
| @@ -57,12 +57,62 @@ def prefix_cache(fs, board_base_path): | |||
| return f"{proto_name}_{base_hash}" | |||
|
|
|||
|
|
|||
| class HashMapper: | |||
There was a problem hiding this comment.
rather than having different classes for each cache type, we can use custom mappers
| super().__init__(*args, **kwargs) | ||
| self.hash_prefix = hash_prefix | ||
| self._mapper = mapper(hash_prefix) | ||
|
|
||
| def hash_name(self, path, *args, **kwargs): |
There was a problem hiding this comment.
the hash_name method is for backwards compatibility with fsspec<2023.9.0
| base_name = super().hash_name(path, same_name) | ||
| suffix = Path(path).name | ||
| return f"{base_name}_{suffix}" | ||
| def hash_name(self, path, *args, **kwargs): |
There was a problem hiding this comment.
the hash_name method is for backwards compatibility with fsspec<2023.9.0
|
Alright, this ended up being some careful cache extraction, but it's in a good place now! Thanks @machow for the pairing sessions to help guide this through. If you have any post-merge questions/comments/concerns, let's make those a follow-up PR 🙌 |
We're using
pinsas a dependency inibis-frameworkand ideally wecan remove the upper-pin on
fsspecso that doesn't impact our users.I think the usage of the moved
hash_namefunction is the only breakingchange between the current upper pin and most recent release, so I've
added a small workaround to handle the change.
All of the local tests pass, I didn't yet set up sufficient credentials
to handle the various cloud tests.