Page MenuHomeSoftware Heritage

Improve the ObjStorage inheritance tree and its documentation
ClosedPublic

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

Details

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 372
Build 549: arc lint + arc unit

Event Timeline

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)
qcampos edited edge metadata.
  • Factorize hash computation into objstorage
  • Rename an argument that didn't follow the API
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 edited edge metadata.
qcampos updated this object.

Remove docstring in subclasses

zack edited edge metadata.
zack added inline comments.
swh/objstorage/objstorage.py
17–25

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 inline comments.
swh/objstorage/objstorage.py
17–25

That's swh.core.hashutil.hashdata.

swh/objstorage/objstorage.py
17–25

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?

swh/objstorage/objstorage.py
17–25

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

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
swh/objstorage/objstorage.py
17–25

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

olasd added a reviewer: olasd.
This revision is now accepted and ready to land.Aug 12 2016, 2:00 PM
This revision was automatically updated to reflect the committed changes.