Page MenuHomeSoftware Heritage

Add Storage.content_get_partition endpoint, to replace content_get_range.
ClosedPublic

Authored by vlorentz on Nov 22 2019, 4:10 PM.

Diff Detail

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont added a subscriber: ardumont.

Looks good ;)

I have mostly concerns about missing types and consistency naming.

swh/storage/api/client.py
58 ↗(On Diff #8064)

'count' instead of limit, you asked me yourself [1]

[1] D2457#57977

swh/storage/in_memory.py
311

type?

313

must be in [0, nb_partitions[
or
must be in [0, nb_partitions - 1]

?

315

or the result order.

328

retrieving

swh/storage/tests/storage_data.py
15

Why do we need this?

(maybe add a comment about it)

swh/storage/utils.py
34

Why if instead of assert as you use sometimes?

45
start = bounds[0].to_bytes(nb_bytes, 'big')
end = None if i == n-1 else bounds[1].to_bytes(nb_bytes, 'big')
return (start, end)

or

start = bounds[0].to_bytes(nb_bytes, 'big')
if i == n-1:
    end = None
else:
    end = bounds[1].to_bytes(nb_bytes, 'big')
return (start, end)
This revision now requires changes to proceed.Dec 16 2019, 9:10 AM
swh/storage/utils.py
34

assert is for things that should never happen, if... raise is for errors that may happen. But yeah sometimes I use assert too much

swh/storage/tests/storage_data.py
15

It's how __getattr__ works: "This method should either return the (computed) attribute value or raise an AttributeError exception." https://docs.python.org/3/reference/datamodel.html#object.__getattr__

Else, getattr() and hasattr() on the object fail, eg.

/usr/lib/python3.7/doctest.py:932: in find
    self._find(tests, obj, name, module, source_lines, globs, {})
.tox/py3/lib/python3.7/site-packages/_pytest/doctest.py:445: in _find
    self, tests, obj, name, module, source_lines, globs, seen
/usr/lib/python3.7/doctest.py:991: in _find
    if ((inspect.isroutine(inspect.unwrap(val))
.tox/py3/lib/python3.7/site-packages/_pytest/doctest.py:408: in _mock_aware_unwrap
    return real_unwrap(obj, stop=_is_mocked)
/usr/lib/python3.7/inspect.py:511: in unwrap
    while _is_wrapper(func):
/usr/lib/python3.7/inspect.py:505: in _is_wrapper
    return hasattr(f, '__wrapped__') and not stop(f)
.tox/py3/lib/python3.7/site-packages/swh/storage/tests/storage_data.py:14: in __getattr__
    v = globals()[key]
E   KeyError: '__wrapped__'
This revision is now accepted and ready to land.Dec 17 2019, 2:22 PM