aai-institute / lakefs-spec

An fsspec implementation for the lakeFS project.
http://lakefs-spec.org/
Apache License 2.0
37 stars 4 forks source link

fs.exists() should return True for common prefixes #274

Closed ozkatz closed 3 months ago

ozkatz commented 3 months ago

Similar to s3fs, fs.exists() should return True also for common prefixes. Ran into this while trying to use lakefs-spec with HuggingFace Datasets - loading a dataset accepts a directory uri, which is checked for existence and subsequently fails.

The check is done in a similar fashion to s3fs' - ensure path ends with '/', attempt listing, return True if there's anything under that prefix.

ozkatz commented 3 months ago

@nicholasjng @AdrianoKF Hope this change meets the contribution guidelines, happy to change as needed.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.37%. Comparing base (7a82c7b) to head (916d912).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #274 +/- ## ========================================== + Coverage 94.32% 94.37% +0.04% ========================================== Files 5 5 Lines 388 391 +3 Branches 72 73 +1 ========================================== + Hits 366 369 +3 Misses 13 13 Partials 9 9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nicholasjng commented 3 months ago

Looks good! I'm only wondering about exceptions - can the exists() call throw in any non-trivial (i.e. other than connection/permission errors) situation, meaning we wouldn't get to decide the prefix question?

ozkatz commented 3 months ago

Thanks @nicholasjng - good question :) I don't think calling objects() increases the surface area for that.

You're already handling server errors, which is the right thing to do - this should also handle any unexpected errors from listing objects.

ozkatz commented 3 months ago

@nicholasjng done :)

nicholasjng commented 3 months ago

Thank you!

ozkatz commented 3 months ago

@nicholasjng any estimation as to when we can expect a next release?

nicholasjng commented 3 months ago

@ozkatz I can release today if it's urgent, but if anything goes wrong, it takes until Tuesday at minimum.

ozkatz commented 3 months ago

Tuesday is absolutely fine :) thank you @nicholasjng !!