search

aai-institute / lakefs-spec

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

Simplify transaction `Placeholder` class #225

Closed maxmynter closed 9 months ago

maxmynter commented 9 months ago

Main Changes

Using wrapt to refactor the Placeholder class with wrapt.ObjectProxy as a transparent proxy.

Remove the need for unwrap via overriding of __getattr__.

Delete unwrap_placeholders utility function.

Make the available method an attribute as it needs no computation input.

Other Changes

Update maintainer e-mails to institute addresses.

Closes https://github.com/aai-institute/lakefs-spec/issues/207

codecov[bot] commented 9 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (d8771bb) 90.05% compared to head (d626af8) 90.56%. Report is 1 commits behind head on main.

Files Patch % Lines
src/lakefs_spec/transaction.py 92.85% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #225 +/- ## ========================================== + Coverage 90.05% 90.56% +0.50% ========================================== Files 7 7 Lines 533 551 +18 Branches 85 93 +8 ========================================== + Hits 480 499 +19 Misses 27 27 + Partials 26 25 -1 ```

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

maxmynter commented 9 months ago

Working with string and representations should work now. I've added the expected argument that enables a more descriptive logging of what the placeholder is placeholding if str or repr is called.

I did not get overriding __instancecheck__ to work as the wrapt object takes precedence. It should somehow be possible. So lets discuss if investing additional time is worth it (plus maybe some pairing).

For now isinstance(sha, Ref) on a placeholder gives False.

We also do not fill the meta_range_id attribute on a Commit from a Placeholder (it is filled when getting it from rev parse).