Details

Please use the same pagination mechanism as the other services, for consistency. eg. https://forge.softwareheritage.org/source/swh-indexer/browse/master/swh/indexer/storage/__init__.py$791-813 and https://forge.softwareheritage.org/source/swh-search/browse/master/swh/search/elasticsearch.py$123-140

Thanks for the links

also, rename limit to count, also for consistency with them.

sure

note that the storage one stills uses limit

backend: Align paginated endpoint consistently with others

(I initially kept the original signature so it did not touch the client part)

This also:

checks consistently the tasks in paginated calls
improve the backend's docstring method

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

Harbormaster completed remote builds in B9764: Diff 8675.Dec 14 2019, 6:31 PM

Fix docstring

swh/scheduler/sql/40-swh-func.sql
309 ↗	(On Diff #8675)	Prior to this, the actual filtering ended up listing tasks completely out of the input range. Because we have tasks with `started` column left null (they never started). Those have the scheduled entry set so we use it for filtering (which was already done client side in the task cli). Now we are consistent. And we will finally be able to filter those out of the scheduler db.

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

Harbormaster completed remote builds in B9765: Diff 8676.Dec 14 2019, 6:37 PM

ardumont mentioned this in D2343: Add Storage.content_get_partition endpoint, to replace content_get_range..Dec 16 2019, 9:10 AM

vlorentz requested changes to this revision.Dec 16 2019, 1:49 PM

vlorentz added inline comments.

swh/scheduler/backend.py
422 ↗	(On Diff #8676)	`page_token` should be an optional string (both for consistency, and be cause we want it to be an opaque token we can change at any time)
swh/scheduler/cli/task.py
539–556 ↗	(On Diff #8676)	The `last_id` as a loop variable should be renamed `page_token`, because it should be an opaque identifier. As for its use as argument of `index_data`, I don't know what to do with it. Maybe add an extra parameter to `filter_task_to_archive`, that is ignored if `page_token` is not None? Or just remove it altogether if we don't actually need it?
tox.ini
9	why?

This revision now requires changes to proceed.Dec 16 2019, 1:49 PM

ardumont added inline comments.Dec 16 2019, 3:39 PM

swh/scheduler/backend.py
422 ↗	(On Diff #8676)	yes, i saw it this morning when reading back your other diffs. I'm ultimately trying to add tests on the other diff and it takes me way too long. i'll get back to that after i'm done with those tests.
swh/scheduler/cli/task.py
539–556 ↗	(On Diff #8676)	we do need it, i'm trying to avoid repeating the while truc loop. Take a look below gen = index_data... if cleanup: # do something with gen else: # do something else with gen
tox.ini
9	Because i'm using tox and ipdb to try and debug the hell of what's happening in there ;) tox -e py3-dev -- -x -s --no-cov --log-level=DEBUG -k test_backend_setup

ardumont added inline comments.Dec 16 2019, 4:14 PM

swh/scheduler/api/client.py
101 ↗	(On Diff #8676)	revert back to limit now.

ardumont added inline comments.Dec 16 2019, 4:31 PM

swh/scheduler/cli/task.py
539–556 ↗	(On Diff #8676)	i meant `while True` ;)

backend: Align paginated endpoint consistently with others:
- Revert to use limit (and not count)
- use optional string type as opaque identifier

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

Harbormaster completed remote builds in B9792: Diff 8705.Dec 16 2019, 4:45 PM

vlorentz accepted this revision.Dec 16 2019, 4:45 PM

vlorentz added inline comments.

swh/scheduler/cli/task.py
558 ↗	(On Diff #8705)	You're still not using it as an opaque identifier if it comes from a CLI option documented as "task id to start from". But I guess there is no easy way other than this one