Details

Reviewers

Group Reviewers

Maniphest Tasks

T403: Improve content archiver testing coverage
T401: Content archiver - Asynchronous version

Commits

rDSTOC4e97e4c687f9: Improve test coverage of the archiver
rDSTOCf74f923a7ecd: Fix the status update of content during archival process
rDSTOCf92426dd8840: Fixes and improvements for T401
rDSTOCfc5ed93e9d45: Extract sql query from archive worker and place it into db module
R65:4e97e4c687f9: Improve test coverage of the archiver
R65:fc5ed93e9d45: Extract sql query from archive worker and place it into db module
R65:f92426dd8840: Fixes and improvements for T401
R65:f74f923a7ecd: Fix the status update of content during archival process
rDARCaf2d9790c4bf: Fixes and improvements for T401
rDOBJS9bd27d2c57a3: Fixes and improvements for T401
rDSTO4e97e4c687f9: Improve test coverage of the archiver
rDSTOf92426dd8840: Fixes and improvements for T401
rDSTOf74f923a7ecd: Fix the status update of content during archival process
rDSTOfc5ed93e9d45: Extract sql query from archive worker and place it into db module

Summary

Ref T401

Fix : The 'ongoing' status wasn't set to the content that were supposed to be

in que archival queue.

Refactor :
- Extract sql query from archive worker and place it into db module
- Refactor in order to improve code quality & separation and to simplify unit testing.
- Change the way the director's configuration is stored to make it easy to send the whole config to ArchiverWorker

Diff Detail

Repository

rDSTO Storage manager

Branch

T403

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 103
Build 146: Software Heritage Python tests
Build 145: arc lint + arc unit

Event Timeline

qcampos updated this revision to Diff 78.May 13 2016, 4:50 PM

qcampos added a task: T401: Content archiver - Asynchronous version.

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)

Herald added a reviewer: Reviewers. · View Herald TranscriptMay 13 2016, 4:50 PM

Improve test coverage of the archiver

Depends on D27

Herald edited edge metadata. · View Herald TranscriptMay 13 2016, 4:54 PM

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 a subscriber: ardumont.May 17 2016, 11:57 AM

ardumont added inline comments.

swh/storage/archiver/director.py
43–54	typo: must contain
48	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.
221	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).
224	...archive_url) of the destination.
225	List of contents
swh/storage/tests/test_archiver.py
138	'enough ' Ctrl-t just before the h in emacs will transpose the blank character and the t ^^