Page MenuHomeSoftware Heritage
Differential D95

Improve the ObjStorage inheritance tree and its documentation
ClosedPublic
Actions

Authored by qcampos on Aug 9 2016, 4:02 PM.

Details

Reviewers
olasd
zack
Group Reviewers
Reviewers
Commits
rDOBJSe3aa5c0a182b: Rename an argument that didn't follow the API
rDOBJS742b86e0b40c: Remove docstring in subclasses
rDOBJS5f2bf871ebe5: Factorize hash computation into objstorage
rDOBJS8de255fb5f54: Use the abc metaclass for some abstract classes
rDOBJSc27165fb372c: Remove duplicate test for the PathSlicingObjectStorage.
rDOBJSd53c6b90f34c: Reference base class in implementations docstrings
Summary
  • The docstring of the ObjStorage implementations have been updated to reference the one present in the base class.
  • Make the base API ObjStorage an Abstract Base Class for python. This add more security when a new ObjStorage implementation is created.
  • Also, the filtering object storage now use this feature to make its extension easier.
  • Remove duplicate test for the PathSlicingObjectStorage.
  • Factorize hash computation into objstorage
  • Rename an argument that didn't follow the API

Diff Detail

Repository
rDOBJS Object storage
Branch
D95-2
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 363
Build 536: arc lint + arc unit

Event Timeline

qcampos updated this revision to Diff 315.Aug 9 2016, 4:02 PM
qcampos retitled this revision from to Improve the ObjStorage inheritance tree and its documentation.
qcampos updated this object.
qcampos edited the test plan for this revision. (Show Details)
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 9 2016, 4:02 PM
qcampos updated this revision to Diff 316.EditedAug 9 2016, 5:30 PM
qcampos edited edge metadata.
  • Factorize hash computation into objstorage
  • Rename an argument that didn't follow the API
Herald edited edge metadata. · View Herald TranscriptAug 9 2016, 5:30 PM
qcampos updated this object.Aug 9 2016, 5:31 PM
qcampos edited edge metadata.
qcampos updated this object.
qcampos updated this object.
zack requested changes to this revision.Aug 10 2016, 10:01 AM
zack added a reviewer: zack.
zack added a subscriber: zack.

Only a minor nitpick about docstrings, the same I made in D94.
The rest looks good.

swh/objstorage/objstorage_pathslicing.py
194–195

Same comment about these docstrings that I made in D94. If they add nothing wrt parent class, please remove them.

This revision now requires changes to proceed.Aug 10 2016, 10:01 AM
qcampos updated this revision to Diff 320.Aug 10 2016, 2:17 PM
qcampos edited edge metadata.
qcampos updated this object.

Remove docstring in subclasses

zack accepted this revision.Aug 12 2016, 10:50 AM
zack edited edge metadata.
zack added inline comments.
swh/objstorage/objstorage.py
17–21

If we really don't have a helper like this one into hashutil, we probably should add one there, and use it here.
There is no need to block this change due to this, but please check if this is the case. If it is, please submit a diff against swh.core to fix this.

This revision is now accepted and ready to land.Aug 12 2016, 10:50 AM
olasd added a subscriber: olasd.Aug 12 2016, 10:56 AM
olasd added inline comments.
swh/objstorage/objstorage.py
17–21

That's swh.core.hashutil.hashdata.

qcampos added inline comments.Aug 12 2016, 12:40 PM
swh/objstorage/objstorage.py
17–21

swh.core.hashutil.hashdata computes the hashcode for each algorithm (sha1, sha256, sha1_git).
Should I use it anyway, or should I make a helper like compute_hash(content, algorithm) in swh.core.hashutil?

olasd added inline comments.Aug 12 2016, 1:17 PM
swh/objstorage/objstorage.py
17–21

You can control which algos hashdata uses with an argument, that should be enough?

qcampos updated this revision to Diff 328.Aug 12 2016, 1:42 PM
qcampos edited edge metadata.
qcampos marked 2 inline comments as done.

Use hashutil.hashdata for hashcode computation in ObjStorage

This revision now requires review to proceed.Aug 12 2016, 1:42 PM
qcampos added inline comments.Aug 12 2016, 1:45 PM
swh/objstorage/objstorage.py
17–21

Didn't see the default arg; that's exactly what's needed!

olasd accepted this revision.Aug 12 2016, 2:00 PM
olasd added a reviewer: olasd.
This revision is now accepted and ready to land.Aug 12 2016, 2:00 PM
Closed by commit rDOBJSd53c6b90f34c: Reference base class in implementations docstrings (authored by qcampos). · Explain WhyAug 12 2016, 2:01 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
swh/
objstorage/
api/
81 lines
multiplexer/
filter/
34 lines
17 lines
1 line
72 lines
36 lines
79 lines
tests/

Diff 320

View Options

swh/objstorage/api/client.py

Loading...
View Options

swh/objstorage/multiplexer/filter/filter.py

Loading...
View Options

swh/objstorage/multiplexer/filter/id_filter.py

Loading...
View Options

swh/objstorage/multiplexer/filter/read_write_filter.py

Loading...
View Options

swh/objstorage/multiplexer/multiplexer_objstorage.py

Loading...
View Options

swh/objstorage/objstorage.py

Loading...
View Options

swh/objstorage/objstorage_pathslicing.py

Loading...
View Options

swh/objstorage/tests/test_objstorage.py

Loading...
About · Developer information · Legal