With no guarantees on the order or how partitioning is done,
and with the new-style pagination.
Details
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
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- content_get_partition
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9332 Build 13658: tox-on-jenkins Jenkins Build 13657: arc lint + arc unit
Event Timeline
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/807/ for more details.
Comment Actions
Looks good ;)
I have mostly concerns about missing types and consistency naming.
| swh/storage/api/client.py | ||
|---|---|---|
| 58 | 'count' instead of limit, you asked me yourself [1] [1] D2457#57977 | |
| swh/storage/in_memory.py | ||
| 307 | type? | |
| 309 | must be in [0, nb_partitions[ ? | |
| 311 | or the result order. | |
| 324 | retrieving | |
| swh/storage/tests/storage_data.py | ||
| 16 | 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 | |
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/841/ for more details.
| swh/storage/tests/storage_data.py | ||
|---|---|---|
| 16 | 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__' | |
Comment Actions
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/842/ for more details.