Details

Reviewers

douardda
olasd
zack
nahimilega
anlambert

Group Reviewers

Reviewers

Commits

rDLS72e208aeedfb: lister_base: Clarify docstrings
rDLS64a9bc691d37: lister.core: Stop creating origins when scheduling tasks

Summary

Prior to this commit, lister did create origins as well in the archive
(swh-storage's db). Now, we plan to only schedule those origins for ingestion
(and we let the loaders do the actual creation).

Test Plan

tox

Then swh-docker-dev through multiple runs.

First run:

checkout origin/master (without this diff)
Add dummy/witness new task type loader-gnu (for the listing to actually succeed, implementation detail of the current lister/loader's status)
Run gnu lister through scheduler
Check swh-scheduler db (new tasks added)
Check swh-storage db (new origins)
Check swh-lister db (filled-in cache)

Second run:

checkout this diff
Add dummy/witness new task type loader-gnu
Run gnu lister through scheduler
Check swh-scheduler db (new tasks added)
Check swh-storage db (no new origins as this behavior got removed here so \m/)
Check swh-lister db (still filled-in cache)

Diff Detail

Repository

rDLS Listers

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 6199
Build 8553: tox-on-jenkins	Jenkins
Build 8552: arc lint + arc unit

Event Timeline

ardumont created this revision.Jun 13 2019, 2:26 PM

Harbormaster failed remote builds in B6193: Diff 5220!Jun 13 2019, 2:26 PM

ardumont retitled this revision from lister.core: Schedule only missing tasks to lister.core: Remove origin creation from lister behavior.Jun 13 2019, 2:38 PM

douardda requested changes to this revision.Jun 13 2019, 2:41 PM

douardda added inline comments.

swh/lister/core/lister_base.py
460	why delete this line from the docstring? Looks like it's still an expected argument to the method.

This revision now requires changes to proceed.Jun 13 2019, 2:41 PM

ardumont added inline comments.Jun 13 2019, 2:53 PM

swh/lister/core/lister_base.py
460	It's a mistep. I actually added it back. I'm also trying to clarify some docstrings.

nahimilega added inline comments.Jun 13 2019, 2:55 PM

swh/lister/core/lister_base.py
387	I think there is no need of this method anymore

ardumont added inline comments.Jun 13 2019, 3:05 PM

swh/lister/core/lister_base.py
387	indeed.

New:

lister_base: Remove no longer used method
lister_base: Clarify docstrings

Rework commit message

lister.core: Stop creating origins when scheduling tasks

Build is green
See https://jenkins.softwareheritage.org/job/DLS/job/tox/179/ for more details.

Harbormaster completed remote builds in B6196: Diff 5223.Jun 13 2019, 3:15 PM

ardumont added inline comments.Jun 13 2019, 3:35 PM

swh/lister/core/lister_base.py
470	As @olasd pointed out, this still feels convoluted. It seems that the current implementation of this method tries to avoid duplication in scheduling tasks. This is or at least should already be taken care of either at the listing time or even at the scheduling time (through the `create_tasks` scheduler api call). This needs some refactoring which i don't want to drag within that diff right now though. The current goal is to deploy this soon and to trigger the listing of gnu and some phabricator instances ;)