Implement more APIs in terms of lakefs abstractions
#240
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
...for `get_file` and `put_file`. Now those methods are truly deferring to their respective superclass methods after the client-side hash check.
Saves an API call by using `WritableObject.copy()` on the resulting object. What remains TBD is whether you can copy files from a reference - the destination seems to always be a branch.
This was causing test failures, so we just defer to the base class now.
Simplifies the code considerably. Unfortunately, it is not yet clear how to list files from commits or tags, since those need different reference abstractions. This is left as a followup for the time being.
The methods are available on the base classes, which means they should be able to be used instead of distinct branch names. However, test coverage might need to be improved, as I do not recall implementing a test for a commit input on any `stat`-like API.Use `lakefs.Reference`s everywhere for object infos The methods are available on the base classes, which means they should be able to be used instead of distinct branch names. However, test coverage might need to be improved, as I do not recall implementing a test for a commit input on any `stat`-like API.
They are distinct types from the lakeFS SDK, so we need to migrate the translator function.
Since the vital methods needed for transactions were only included in v0.2.0, this change incurs a mandatory upgrade to lakefs v0.2.0. The implicit branch creation is included in the `mode != "rb"` branch in `fs._open()`. The migration required the overload of `tail()`, since the `Reader` and `Writer` do not have a size attribute.
Since we do not inherit from `AbstractBufferedFile` any longer, including the tests makes no sense.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
+ Coverage 91.19% 93.34% +2.15%
==========================================
Files 5 5
Lines 477 406 -71
Branches 82 68 -14
==========================================
- Hits 435 379 -56
+ Misses 24 18 -6
+ Partials 18 9 -9 ☔ View full report in Codecov by Sentry. |
This has been superseded by `fs.open()`, which has pre-sign support for uploads implemented through urllib3.
Those errors are all caught in `lakefs`, which uses a custom translator. Hence, we can expect to only get `ServerException`s from SDK functions.
This was referenced Dec 28, 2023
Closed
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.
Contains implementations for
ls, exists, info, cp(_file), rm(_file).Residual test failures are because the
LakeFSFilehas not been migrated yet.