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

Modify error translator to raise more informative errors #190

Closed nicholasjng closed 7 months ago

nicholasjng commented 7 months ago

Prepares the status, reason, and filename arguments unconditionally for each observed API error.

This way, users who see translated errors actually know what the filename involved is instead of just getting the (less than ideal) lakeFS API reason.

One particular behavior is that of fs.ls() in case the branch/ref is not found: Here, unlike in the repository case, the branch is not part of the error message, which means just a generic "file not found" error.

We use OS Errnos for more exotic statuscodes, and HTTP codes for 401, 403 and 404 errors.

codecov[bot] commented 7 months ago

Codecov Report

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

Comparison is base (bcb7008) 87.36% compared to head (6ee0c4d) 88.05%. Report is 1 commits behind head on main.

Files Patch % Lines
src/lakefs_spec/spec.py 87.50% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #190 +/- ## ========================================== + Coverage 87.36% 88.05% +0.68% ========================================== Files 7 7 Lines 475 477 +2 Branches 67 69 +2 ========================================== + Hits 415 420 +5 + Misses 34 31 -3 Partials 26 26 ```

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

nicholasjng commented 7 months ago

Repro script:

def main():
    fs = LakeFSFileSystem()
    fs.get_file("quickstart/hello/hello.py", "hello.py")

On main, this results in:

FileNotFoundError: HTTP404 (Not Found): not found

While on this PR branch:

FileNotFoundError: 404 not found: 'quickstart/hello/hello.py'
nicholasjng commented 7 months ago

Since the distinction between HTTP status codes and OS Errnos is completely arbitrary here: I'm also completely fine (and actually happier) with changing all error codes over to either type (HTTP codes vs. OS error codes), but they should be consistent.

nicholasjng commented 7 months ago

What's also possible is to let both appear in the error message, e.g. like so:

# errno 2 is ENOENT
FileNotFoundError: [Errno 2] 404 not found: 'quickstart/hello/hello.py'

Unfortunately I have not found a way to omit the actual errno bracket in the string representation if reason and filename are given.

EDIT: Collapsed all arguments into a single string to omit the errno bracket in b676945.