search

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

(docs): Add lakeFS-spec and lakeFS-SDK transaction differences #252

Closed maxmynter closed 5 months ago

maxmynter commented 5 months ago

Closes https://github.com/aai-institute/MLOps-Engineering/issues/29

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (15c3526) 94.10% compared to head (78de589) 94.08%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #252 +/- ## ========================================== - Coverage 94.10% 94.08% -0.02% ========================================== Files 5 5 Lines 407 406 -1 Branches 72 72 ========================================== - Hits 383 382 -1 Misses 15 15 Partials 9 9 ```

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

maxmynter commented 5 months ago

I cannot directly comment on code not (yet) affected by this PR.

In the complete function of the LakeFSTransaction context class ( line 162 in the transactions.py ) we set self.fs._intrans = False. Is that necessary, since the __exit__ function of the Transaction parent class from fsspec already sets this flag to false after calling complete?

nicholasjng commented 5 months ago

In the complete function of the LakeFSTransaction context class ( line 162 in the transactions.py ) we set self.fs._intrans = False. Is that necessary, since the __exit__ function of the Transaction parent class from fsspec already sets this flag to false after calling complete?

I guess it's not necessary, but it does not hurt, either, so I'm unsure. If you can verify that this does not break any tests, I'm happy to see it removed.