With no guarantees on the order or how partitioning is done,
and with the new-style pagination.
Details
- Reviewers
ardumont - Group Reviewers
Reviewers - Maniphest Tasks
- T1912: Support origin pagination without origin ids
- Commits
- rDSTOCfdf2a3c697d8: Add Storage.content_get_partition endpoint, to replace content_get_range.
rDSTOfdf2a3c697d8: Add Storage.content_get_partition endpoint, to replace content_get_range.
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- content_get_partition
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9840 Build 14522: tox-on-jenkins Jenkins Build 14521: arc lint + arc unit
Event Timeline
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/807/ for more details.
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[ ? | |
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) |
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 |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/841/ for more details.
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__' |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/842/ for more details.