Skip to content

Forbidden 403 on missing chunk in remote Zarr #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joshmoore opened this issue Jun 30, 2020 · 12 comments
Closed

Forbidden 403 on missing chunk in remote Zarr #342

joshmoore opened this issue Jun 30, 2020 · 12 comments

Comments

@joshmoore
Copy link
Contributor

Both direct HTTP access and S3FileSystem access of an S3 store fail with a PermissionsError if a Zarr chunk does not exist.

s3fs details
In [3]: def load_binary_from_s3(image_id, resolution='0'):
   ...:     cache_size_mb = 2048
   ...:     cfg = {
   ...:         'anon': True,
   ...:         'client_kwargs': {
   ...:             'endpoint_url': 'https://s3.embassy.ebi.ac.uk',
   ...:         },
   ...:         'root': 'idr/zarr/v0.1/%s.zarr/%s/' % (image_id, resolution)
   ...:     }
   ...:     import s3fs
   ...:     s3 = s3fs.S3FileSystem(
   ...:         anon=cfg['anon'],
   ...:         client_kwargs=cfg['client_kwargs'],
   ...:     )
   ...:     store = s3fs.S3Map(root=cfg['root'], s3=s3, check=False)
   ...:     import dask.array as da
   ...:     return da.from_zarr(store)
   ...:

In [4]:

In [4]: x = load_binary_from_s3(9836950, "masks/0")

In [5]: x
Out[5]: dask.array<from-zarr, shape=(1, 1, 156, 816, 1636), dtype=int64, chunksize=(1, 1, 1, 816, 1636), chunktype=numpy.ndarray>

In [6]: x.compute()

----> 1 x.compute()

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/base.py in compute(self, **kwargs)
    164         dask.base.compute
    165         """
--> 166         (result,) = compute(self, traverse=False, **kwargs)
    167         return result
    168

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/base.py in compute(*args, **kwargs)
    442         postcomputes.append(x.__dask_postcompute__())
    443
--> 444     results = schedule(dsk, keys, **kwargs)
    445     return repack([f(r, *a) for r, (f, a) in zip(results, postcomputes)])
    446

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/threaded.py in get(dsk, result, cache, num_workers, pool, **kwargs)
     82         get_id=_thread_get_id,
     83         pack_exception=pack_exception,
---> 84         **kwargs
     85     )
     86

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in get_async(apply_async, num_workers, dsk, result, cache, get_id, rerun_exceptions_locally, pack_exception, raise_exception, callbacks, dumps, loads, **kwargs)
    484                         _execute_task(task, data)  # Re-execute locally
    485                     else:
--> 486                         raise_exception(exc, tb)
    487                 res, worker_id = loads(res_info)
    488                 state["cache"][key] = res

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in reraise(exc, tb)
    314     if exc.__traceback__ is not tb:
    315         raise exc.with_traceback(tb)
--> 316     raise exc
    317
    318

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in execute_task(key, task_info, dumps, loads, get_id, pack_exception)
    220     try:
    221         task, data = loads(task_info)
--> 222         result = _execute_task(task, data)
    223         id = get_id()
    224         result = dumps((result, id))

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/core.py in _execute_task(arg, cache, dsk)
    119         # temporaries by their reference count and can execute certain
    120         # operations in-place.
--> 121         return func(*(_execute_task(a, cache) for a in args))
    122     elif not ishashable(arg):
    123         return arg

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/array/core.py in getter(a, b, asarray, lock)
     96         lock.acquire()
     97     try:
---> 98         c = a[b]
     99         if asarray:
    100             c = np.asarray(c)

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in __getitem__(self, selection)
    570
    571         fields, selection = pop_fields(selection)
--> 572         return self.get_basic_selection(selection, fields=fields)
    573
    574     def get_basic_selection(self, selection=Ellipsis, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in get_basic_selection(self, selection, out, fields)
    696         else:
    697             return self._get_basic_selection_nd(selection=selection, out=out,
--> 698                                                 fields=fields)
    699
    700     def _get_basic_selection_zd(self, selection, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _get_basic_selection_nd(self, selection, out, fields)
    738         indexer = BasicIndexer(selection, self)
    739
--> 740         return self._get_selection(indexer=indexer, out=out, fields=fields)
    741
    742     def get_orthogonal_selection(self, selection, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _get_selection(self, indexer, out, fields)
   1026             # load chunk selection into output array
   1027             self._chunk_getitem(chunk_coords, chunk_selection, out, out_selection,
-> 1028                                 drop_axes=indexer.drop_axes, fields=fields)
   1029
   1030         if out.shape:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _chunk_getitem(self, chunk_coords, chunk_selection, out, out_selection, drop_axes, fields)
   1584         try:
   1585             # obtain compressed data for chunk
-> 1586             cdata = self.chunk_store[ckey]
   1587
   1588         except KeyError:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/mapping.py in __getitem__(self, key, default)
     73         k = self._key_to_str(key)
     74         try:
---> 75             result = self.fs.cat(k)
     76         except (FileNotFoundError, IsADirectoryError, NotADirectoryError):
     77             if default is not None:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/spec.py in cat(self, path)
    585     def cat(self, path):
    586         """ Get the content of a file """
--> 587         return self.open(path, "rb").read()
    588
    589     def get(self, rpath, lpath, recursive=False, **kwargs):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/spec.py in open(self, path, mode, block_size, cache_options, **kwargs)
    773                 autocommit=ac,
    774                 cache_options=cache_options,
--> 775                 **kwargs
    776             )
    777             if not ac:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/s3fs/core.py in _open(self, path, mode, block_size, acl, version_id, fill_cache, cache_type, autocommit, requester_pays, **kwargs)
    376                       version_id=version_id, fill_cache=fill_cache,
    377                       s3_additional_kwargs=kw, cache_type=cache_type,
--> 378                       autocommit=autocommit, requester_pays=requester_pays)
    379
    380     def _lsdir(self, path, refresh=False, max_items=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/s3fs/core.py in __init__(self, s3, path, mode, block_size, acl, version_id, fill_cache, s3_additional_kwargs, autocommit, cache_type, requester_pays)
   1095         self.req_kw = {'RequestPayer': 'requester'} if requester_pays else {}
   1096         super().__init__(s3, path, mode, block_size, autocommit=autocommit,
-> 1097                          cache_type=cache_type)
   1098         self.s3 = self.fs  # compatibility
   1099         if self.writable():

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/spec.py in __init__(self, fs, path, mode, block_size, autocommit, cache_type, cache_options, **kwargs)
   1063         if mode == "rb":
   1064             if not hasattr(self, "details"):
-> 1065                 self.details = fs.info(path)
   1066             self.size = self.details["size"]
   1067             self.cache = caches[cache_type](

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/s3fs/core.py in info(self, path, version_id, refresh)
    546                     return super(S3FileSystem, self).info(path)
    547                 else:
--> 548                     raise ee
    549             except ParamValidationError as e:
    550                 raise ValueError('Failed to head path %r: %s' % (path, e))

PermissionError: Forbidden
http details
In [1]: import dask.array as da

In [2]: x = da.from_zarr("https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/9836950.zarr/masks/0")

In [3]: x.compute()
---------------------------------------------------------------------------
HTTPError                                 Traceback (most recent call last)
<ipython-input-3-ef36793348c2> in <module>
----> 1 x.compute()

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/base.py in compute(self, **kwargs)
    164         dask.base.compute
    165         """
--> 166         (result,) = compute(self, traverse=False, **kwargs)
    167         return result
    168

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/base.py in compute(*args, **kwargs)
    442         postcomputes.append(x.__dask_postcompute__())
    443
--> 444     results = schedule(dsk, keys, **kwargs)
    445     return repack([f(r, *a) for r, (f, a) in zip(results, postcomputes)])
    446

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/threaded.py in get(dsk, result, cache, num_workers, pool, **kwargs)
     82         get_id=_thread_get_id,
     83         pack_exception=pack_exception,
---> 84         **kwargs
     85     )
     86

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in get_async(apply_async, num_workers, dsk, result, cache, get_id, rerun_exceptions_locally, pack_exception, raise_exception, callbacks, dumps, loads, **kwargs)
    484                         _execute_task(task, data)  # Re-execute locally
    485                     else:
--> 486                         raise_exception(exc, tb)
    487                 res, worker_id = loads(res_info)
    488                 state["cache"][key] = res

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in reraise(exc, tb)
    314     if exc.__traceback__ is not tb:
    315         raise exc.with_traceback(tb)
--> 316     raise exc
    317
    318

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/local.py in execute_task(key, task_info, dumps, loads, get_id, pack_exception)
    220     try:
    221         task, data = loads(task_info)
--> 222         result = _execute_task(task, data)
    223         id = get_id()
    224         result = dumps((result, id))

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/core.py in _execute_task(arg, cache, dsk)
    119         # temporaries by their reference count and can execute certain
    120         # operations in-place.
--> 121         return func(*(_execute_task(a, cache) for a in args))
    122     elif not ishashable(arg):
    123         return arg

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/dask/array/core.py in getter(a, b, asarray, lock)
     96         lock.acquire()
     97     try:
---> 98         c = a[b]
     99         if asarray:
    100             c = np.asarray(c)

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in __getitem__(self, selection)
    570
    571         fields, selection = pop_fields(selection)
--> 572         return self.get_basic_selection(selection, fields=fields)
    573
    574     def get_basic_selection(self, selection=Ellipsis, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in get_basic_selection(self, selection, out, fields)
    696         else:
    697             return self._get_basic_selection_nd(selection=selection, out=out,
--> 698                                                 fields=fields)
    699
    700     def _get_basic_selection_zd(self, selection, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _get_basic_selection_nd(self, selection, out, fields)
    738         indexer = BasicIndexer(selection, self)
    739
--> 740         return self._get_selection(indexer=indexer, out=out, fields=fields)
    741
    742     def get_orthogonal_selection(self, selection, out=None, fields=None):

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _get_selection(self, indexer, out, fields)
   1026             # load chunk selection into output array
   1027             self._chunk_getitem(chunk_coords, chunk_selection, out, out_selection,
-> 1028                                 drop_axes=indexer.drop_axes, fields=fields)
   1029
   1030         if out.shape:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/zarr/core.py in _chunk_getitem(self, chunk_coords, chunk_selection, out, out_selection, drop_axes, fields)
   1584         try:
   1585             # obtain compressed data for chunk
-> 1586             cdata = self.chunk_store[ckey]
   1587
   1588         except KeyError:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/mapping.py in __getitem__(self, key, default)
     73         k = self._key_to_str(key)
     74         try:
---> 75             result = self.fs.cat(k)
     76         except (FileNotFoundError, IsADirectoryError, NotADirectoryError):
     77             if default is not None:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/implementations/http.py in cat(self, url)
    108     def cat(self, url):
    109         r = self.session.get(url, **self.kwargs)
--> 110         r.raise_for_status()
    111         return r.content
    112

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/requests/models.py in raise_for_status(self)
    939
    940         if http_error_msg:
--> 941             raise HTTPError(http_error_msg, response=self)
    942
    943     def close(self):

HTTPError: 403 Client Error: Forbidden for url: https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/9836950.zarr/masks/0/0.0.7.0.0

The server is known to be quite restrictive, disallowing directory listings, etc. The only workaround I can think of is to create the missing chunks with the fill value which I'd like to avoid since this will be repeated for millions of images.

@martindurant
Copy link
Member

This was a deliberate decision, so that (intermittent) connection errors do not show up as false FileNotFound errors, which would mean chunks being filled by the default fill value.
#259 implemented this behaviour
#255 reported the problem

I can't think of an easy way to make our implementation more flexible in this regard: not being able to access a file is not the same as the file not existing. There could be a flag to the mapper to translate any fetch error to KeyError at the user's choice, which would risk the zarr problems from before.

@joshmoore
Copy link
Contributor Author

joshmoore commented Jun 30, 2020

Hmmm..... thanks, @martindurant. cc'ing @alimanfoo while I do some more reading.

Edit: In most of the text of #255, Alistair mentions "transient exceptions" except for in the one sentence

to distinguish between a genuine "this file/object does not exist" error and any other kind of error.

You go on to mention permissions error, @martindurant. I think this is a request for such permission errors to not be handled as transient errors.

@alimanfoo
Copy link

FWIW I think I'd suggest fsspec is doing the right thing in principle here, in the sense that it should only raise a KeyError when it definitely knows that a file does not exist. I.e., it can translate FileNotFound -> KeyError, but any other type of exception should be propagated as-is.

But it seems weird that the service is responding with a 403 here, you don't need to list the bucket, you're just attempting to read an object that doesn't exist, surely 404 would be the right response.

@joshmoore
Copy link
Contributor Author

you're just attempting to read an object that doesn't exist, surely 404 would be the right response

I'll look into the "spec" (or at least compare how other implementations handle this), but to some degree, I'd concur with the 403 -- by telling someone a key doesn't exist the server would be giving away some knowledge of the underlying system but it has no idea who I am and therefore shouldn't tell me anything.

@alimanfoo
Copy link

but to some degree, I'd concur with the 403 -- by telling someone a key doesn't exist the server would be giving away some knowledge of the underlying system but it has no idea who I am and therefore shouldn't tell me anything

FWIW you can argue it the other way around. E.g., GitHub gives you a 404 if you try to access a resource that you don't have permission to.

@joshmoore
Copy link
Contributor Author

Oh, granted. Guess that's my point: there are arguments both ways. Current status of research:

  • minio with {"Version":"2012-10-17","Statement":[{"Action":["s3:ListBucket"],"Effect":"Deny","Principal":{"AWS":["*"]},"Resource":["arn:aws:s3:::6001240.zarr/*"],"Sid":""},{"Action":["s3:GetObject"],"Effect":"Allow","Principal":{"AWS":["*"]},"Resource":["arn:aws:s3:::6001240.zarr/*"],"Sid":""}]} returns 404
  • AWS with no policy set up returns 403 "AccessDenied"
  • Setting up AWS policies is a pain.... so I haven't yet applied the above to AWS put I'll keep tinkering.

However, I do wonder if it doesn't suffice that 403 is not transient, since that is what you were concerned about.

(https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html certainly lists 403, but when is less clear....)

@alimanfoo
Copy link

Yeah, I guess this means that, whether you receive a 403 or 404, in both cases the true error could be either file not found or bad permissions. Ugh. Makes it hard to figure out what fsspec and zarr should do. The thing I'm most concerned about is, when is it safe for zarr to assume a chunk doesn't exist and fill it with missing values.

@joshmoore
Copy link
Contributor Author

For what it's worth,

    def cat(self, url):
        if not self.exists(url):         ## workaround
            raise KeyError("Unknown")    ## 
        r = self.session.get(url, **self.kwargs)
        r.raise_for_status()
        return r.content

at https://github.com/intake/filesystem_spec/blob/c1c1176bf53eedc7d3d0e0ff1ecf07eb2e53d86a/fsspec/implementations/http.py#L108 let's the dask array computation continue.

cc: @will-moore

@joshmoore
Copy link
Contributor Author

joshmoore commented Aug 14, 2020

Ping @martindurant @alimanfoo -- Sorry, I let this slip over the summer slump (and was just *cough* kindly reminded by a user) Any thoughts on what can be done here?

cc: @jni

@joshmoore
Copy link
Contributor Author

Also an update that I just realized, in @jni's bug report, the error is a 404:

    raise ClientResponseError(
aiohttp.client_exceptions.ClientResponseError: 404, message='Not Found', url=URL('https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr/masks/0/0.0.0.0.0')

I imagine the change from 403 to 404 stems from this bucket recently being made public.

code:

In [3]: import dask.array as da
In [3]: da.from_zarr("https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr/masks/0").compute()

stack trace:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/mapping.py in __getitem__(self, key, default)
     73         k = self._key_to_str(key)
     74         try:
---> 75             result = self.fs.cat(k)
     76         except (FileNotFoundError, IsADirectoryError, NotADirectoryError,):
     77             if default is not None:

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/fsspec/implementations/http.py in cat(self, url)
    110         #    raise KeyError("JOSH")
    111         r = self.session.get(url, **self.kwargs)
--> 112         r.raise_for_status()
    113         return r.content
    114

/usr/local/anaconda3/envs/demo/lib/python3.6/site-packages/requests/models.py in raise_for_status(self)
    939
    940         if http_error_msg:
--> 941             raise HTTPError(http_error_msg, response=self)
    942
    943     def close(self):

HTTPError: 404 Client Error: Not Found for url: https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr/masks/0/0.0.7.0.0

joshmoore added a commit to joshmoore/ome-zarr-py that referenced this issue Aug 14, 2020
Primarily driven by masks with missing chunks which
currently lead to failures (see issue below) but also
by a larger number of masks leading to performance
issues, default to having mask labels added by set
to invisible.

see: fsspec/filesystem_spec#342 (comment)
@martindurant
Copy link
Member

martindurant commented Aug 14, 2020

A 404 should definitely become FileNotFound

@martindurant
Copy link
Member

Note that zarr-developers/zarr-python#546 proposes making the list of exceptions to be regarded as "missing" (i.e., KeyError) configurable in the storage driver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants