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

Writing to protected branches produces a `lakefs.ForbiddenException` instead of a `PermissionError` #296

Open nicholasjng opened 1 week ago

nicholasjng commented 1 week ago

What happened?

We have an exception translator that translates lakefs API exceptions to builtin Python exceptions to ensure conformity of our fsspec implementation.

I just tried streaming JSON data directly into lakeFS for our nnbench project with a command like this:

nnbench .sandbox/example.py -o "lakefs://quickstart/test/res.json"

Details are not as important, it just calls fs.open() under the hood and passes the resulting file descriptor off to json.dump(). When attempting to write that file in a fresh lakeFS quickstart repo like above, I get the following exception:

``` Traceback (most recent call last): File "/Users/nicholasjunge/Workspaces/python/nnbench/.venv/bin/nnbench", line 8, in sys.exit(main()) ^^^^^^ File "/Users/nicholasjunge/Workspaces/python/nnbench/src/nnbench/cli.py", line 64, in main f.write(record, outfile) File "/Users/nicholasjunge/Workspaces/python/nnbench/src/nnbench/reporter/file.py", line 288, in write with fd as fp: File "/Users/nicholasjunge/Workspaces/python/nnbench/.venv/lib/python3.11/site-packages/lakefs/object.py", line 162, in __exit__ self.close() File "/Users/nicholasjunge/Workspaces/python/nnbench/.venv/lib/python3.11/site-packages/lakefs/object.py", line 487, in close stats = self._upload_presign() if self.pre_sign else self._upload_raw() ^^^^^^^^^^^^^^^^^^ File "/Users/nicholasjunge/Workspaces/python/nnbench/.venv/lib/python3.11/site-packages/lakefs/object.py", line 540, in _upload_raw handle_http_error(resp) File "/Users/nicholasjunge/Workspaces/python/nnbench/.venv/lib/python3.11/site-packages/lakefs/exceptions.py", line 159, in handle_http_error raise lakefs_ex lakefs.exceptions.ForbiddenException: code: 403, reason: Forbidden, body: {'message': 'cannot write to protected branch'} ```

So it seems that we're missing a raise translate_lakefs_error() somewhere in LakeFSFileSystem.open().

What did you expect to happen?

I expected something like PermissionError: cannot write to protected branch 'main', but the branch name would probably be sugar on top in our exception facility. There have been a few instances where I wanted more detail from lakeFS exceptions, but some (or all) of it is just the scarcity of info you get from the server.

lakeFS-spec version

0.11.0

lakeFS version

1.42.0

nicholasjng commented 1 week ago

This might be a little harder than I thought - we're just handing off the lakefs.ObjectReader (or Writer) to the user, and that is what produces the error in its .close() method when trying to send the buffer to the server.

Since the user voluntarily takes ownership of these object readers and writers when using them outside of a context manager (or even within? I don't see error translating anywhere here tbh), there might not be a lot we can do here, since we're not "within" our file system anymore.

EDIT: At least not without subclassing the ObjectReader and Writer classes to add something to the .close().