Page MenuHomeSoftware Heritage

Fixes and improvements for T401
ClosedPublic

Authored by qcampos on May 13 2016, 4:50 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

qcampos updated this revision to Diff 78.May 13 2016, 4:50 PM
qcampos retitled this revision from to Fixes and improvements for T401.
qcampos updated this object.
qcampos edited the test plan for this revision. (Show Details)
qcampos updated this revision to Diff 79.May 13 2016, 4:54 PM
qcampos edited edge metadata.
  • Improve test coverage of the archiver

Depends on D27

I just tried to use a "Depends on" keyword in diff summary in order to create a second diff D28 relative to D27. Didn't work as expected, it's just do the same than arc diff --update D27.
However, the last commit that improve the test coverage can be reviewed separately but depends strongly on the other fixes and refactoring.

ardumont added inline comments.
swh/storage/archiver/director.py
43–54

typo: must contain

49

typo: to copy all

swh/storage/archiver/worker.py
29–30

s/where is the content location/(db connection url, content location)/

31–32

For the sake of coherence with the rest of your diff, you forgot to adapt the docstring on config argument.

52

arguments.

217–239

If one copier fails significantly badly (e.g. by throwing something), the remaining copier won't even be triggered.
We may want to trigger the other ones even though one or more fail.
This could dealt with in the run_copier method.

222

You might want to add in the docstring the fact that if the copier fails to run (returns False), we consider the content not being copied (because no update takes place).

225

...archive_url) of the destination.

226

List of contents

swh/storage/tests/test_archiver.py
129

'enough '
Ctrl-t just before the h in emacs will transpose the blank character and the t ^^

qcampos updated this revision to Diff 80.May 17 2016, 12:21 PM
qcampos marked 10 inline comments as done.
qcampos edited edge metadata.

Correct some typo and an uncatched exception.

This comment was removed by qcampos.
qcampos updated this revision to Diff 81.May 17 2016, 12:36 PM
qcampos edited edge metadata.

More typo corrections

ardumont accepted this revision.May 17 2016, 12:39 PM
ardumont added a reviewer: ardumont.
This revision is now accepted and ready to land.May 17 2016, 12:39 PM
qcampos updated this revision to Diff 82.EditedMay 17 2016, 12:39 PM
qcampos edited edge metadata.

Last typo I hope

This revision now requires review to proceed.May 17 2016, 12:39 PM
ardumont accepted this revision.May 17 2016, 12:40 PM
ardumont edited edge metadata.
This revision is now accepted and ready to land.May 17 2016, 12:40 PM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
zack edited edge metadata.May 30 2016, 3:20 PM
zack changed the visibility from "All Users" to "Public (No Login Required)".