Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T645: Type swh-storage endpoints with swh.model objects

Commits

rDSCH642620845ac2: cli.task: Migrate scheduler cli to latest storage change on iter_origins

Summary

Drop --max-id, --min-id cli parameter which can no longer work
Expose --page-token and --limit instead

Related to T645

Test Plan

tox

Diff Detail

Repository

rDSCH Scheduling utilities

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 14211
Build 21848: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 21847: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	160 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_cli::test_task_schedule_origins swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7fe047842630> storage = <swh.storage.in_memory.InMemoryStorage object at 0x7fe047842a58>
	432 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_cli::test_task_schedule_origins_dry_run swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7fe0478afbe0> storage = <swh.storage.in_memory.InMemoryStorage object at 0x7fe0477c2048>
	363 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_cli::test_task_schedule_origins_kwargs swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7fe047646cf8> storage = <swh.storage.in_memory.InMemoryStorage object at 0x7fe0476460b8>
	463 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_cli::test_task_schedule_origins_with_limit swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7fe0479f0c88> storage = <swh.storage.in_memory.InMemoryStorage object at 0x7fe0479f04a8>
	363 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.scheduler.tests.test_cli::test_task_schedule_origins_with_page_token swh_scheduler = <swh.scheduler.backend.SchedulerBackend object at 0x7fe04658dac8> storage = <swh.storage.in_memory.InMemoryStorage object at 0x7fe04658dba8>
		View Full Test Results (5 Failed · 117 Passed · 1 Skipped)

Event Timeline

ardumont created this revision.Aug 1 2020, 10:04 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptAug 1 2020, 10:04 AM

ardumont edited the summary of this revision. (Show Details)Aug 1 2020, 10:06 AM

Build has FAILED

Patch application report for D3682 (id=12964)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit e48885ce005c8bbf27bf3fc85883489248af7a31
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/50/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/50/console

Harbormaster failed remote builds in B14202: Diff 12964!Aug 1 2020, 10:07 AM

vlorentz added a subscriber: vlorentz.Aug 1 2020, 10:08 AM

vlorentz added inline comments.

swh/scheduler/cli/task.py
289–291	why not use islice directly?
360	wrong order of args btw

swh/scheduler/cli/task.py
350–354	I don't understand the issue. Just change the arguments to take the `page_token` as argument instead of `min_id`

ardumont added inline comments.Aug 1 2020, 10:12 AM

swh/scheduler/cli/task.py
289–291	yeah, i initially took it from the doc on itertools [1] and then thought to use it directly the generator... [1] https://docs.python.org/3/library/itertools.html#itertools-recipes
360	because my brain wanted to use islice all along ;)

ardumont edited the summary of this revision. (Show Details)Aug 1 2020, 10:13 AM

ardumont added inline comments.

swh/scheduler/cli/task.py
350–354	well for one, `iter_origins` does not return any page_token after that...

vlorentz added inline comments.Aug 1 2020, 10:33 AM

swh/scheduler/cli/task.py
350–354	Hmmm... We could write a variant of `iter_origins` that yields tokens in addition to the origins.

ardumont added inline comments.Aug 1 2020, 10:35 AM

swh/scheduler/cli/task.py
350–354	or drop iter_origins and use directly storage.origin_list which does just that...?

vlorentz added inline comments.Aug 1 2020, 10:37 AM

swh/scheduler/cli/task.py
350–354	yeah, indeed

ardumont added inline comments.Aug 1 2020, 10:39 AM

swh/scheduler/cli/task.py
350–354	that'd simplify even further D3681 either by: dropping `iter_origins` altogether simplifying its definition to reuse directly swh.core.api.classes.stream_results (and stop exposing the page_token)

ardumont planned changes to this revision.Aug 1 2020, 10:46 AM

Amend according to discussions
Add missing test which fails due to a most probably wrong origin-list implementation for the in-memory storage

ardumont planned changes to this revision.Aug 1 2020, 11:04 AM

ardumont edited the summary of this revision. (Show Details)

Build has FAILED

Patch application report for D3682 (id=12965)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit e4e94a48f184164e015f173027a3c471dfc21389
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/51/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/51/console

Harbormaster failed remote builds in B14203: Diff 12965!Aug 1 2020, 11:07 AM

ardumont mentioned this in D3683: storage.in_memory: Fix origin_list implementation.Aug 1 2020, 11:16 AM

Add scenario with page-token

tests need storage > v0.11.8 to pass

ardumont edited the test plan for this revision. (Show Details)Aug 1 2020, 11:28 AM

Build has FAILED

Patch application report for D3682 (id=12968)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit 95882a4212324f828a13c19ddc0ed8284e93362b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/53/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/53/console

Harbormaster failed remote builds in B14206: Diff 12968!Aug 1 2020, 11:31 AM

ardumont marked 2 inline comments as done.Aug 1 2020, 2:49 PM

ardumont edited the summary of this revision. (Show Details)Aug 1 2020, 7:03 PM

ardumont edited the test plan for this revision. (Show Details)

Fix tests appropriately

Build has FAILED

Patch application report for D3682 (id=12972)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit ba058084f2e403a9c13371b51eff629f228ea23a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/54/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/54/console

Harbormaster failed remote builds in B14210: Diff 12972!Aug 1 2020, 7:04 PM

ardumont edited the test plan for this revision. (Show Details)Aug 1 2020, 7:07 PM

Fix flake8 warnings, turns out we can compose f and r prefix for strings (cool)

tests still failing since they still need a storage bump

Build has FAILED

Patch application report for D3682 (id=12973)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit 7db78dd9bf7a3cdba950094ce850fb6144dedbaa
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/55/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/55/console

Harbormaster failed remote builds in B14211: Diff 12973!Aug 1 2020, 7:12 PM

It would be nice to print the token from time to time so the process can be resumed if interrupted, but that's good enough for now

This revision is now accepted and ready to land.Aug 3 2020, 9:13 AM

It would be nice to print the token from time to time so the process can be resumed if interrupted, but that's good enough for now

Yeah, it hit me during the night that i forgot that.
How do you envision this, is a logger.info instruction enough?

I was thinking of print(), but that would work too

ardumont mentioned this in D3681: storage*: Drop origin-get-range in favor of origin-list.Aug 3 2020, 10:38 AM

I was thinking of print(), but that would work too

yeah, ok but now i'm wondering where.
I suppose it's common enough it should go in swh.core.api.classes.stream_results

If not, then i suppose, this can go in the iter_origins (from the storage diff)?

Adapt according to review/discussion, inline iter_origins within the scheduler
so it displays the page_token during pagination.

Tests should be fine now as latest storage got bumped

ardumont edited the test plan for this revision. (Show Details)Aug 3 2020, 12:12 PM

Build is green

Patch application report for D3682 (id=12989)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit 2cf14951a7fc4f2a68cae9e80a9f352f6e27495c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/56/ for more details.

Harbormaster completed remote builds in B14230: Diff 12989.Aug 3 2020, 12:13 PM

vlorentz added inline comments.Aug 3 2020, 12:14 PM

swh/scheduler/cli/task.py
292	"Eventually" does not mean "éventuellement", but "finalement" ;) Use "optionally" instead

Rework docstring sentences
(thx for the heads up)

Build is green

Patch application report for D3682 (id=12990)

Rebasing onto 849d063758...

Current branch diff-target is up to date.

Changes applied before test

commit 642620845ac21c4b3fbb0aae69befc87c5ac64be
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Aug 1 10:03:23 2020 +0200

    cli.task: Migrate scheduler cli to latest storage change on iter_origins
    
    Related to T645

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/57/ for more details.

Harbormaster completed remote builds in B14231: Diff 12990.Aug 3 2020, 12:20 PM

vlorentz accepted this revision.Aug 3 2020, 12:24 PM

Closed by commit rDSCH642620845ac2: cli.task: Migrate scheduler cli to latest storage change on iter_origins (authored by ardumont). · Explain WhyAug 3 2020, 12:24 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDSCH642620845ac2: cli.task: Migrate scheduler cli to latest storage change on iter_origins.

cli.task: Migrate scheduler cli to latest storage change on iter_origins
ClosedPublic
Actions

Details

Diff Detail

Unit TestsFailed
View All

Event Timeline

Patch application report for D3682 (id=12964)

Changes applied before test

Patch application report for D3682 (id=12965)

Changes applied before test

Patch application report for D3682 (id=12968)

Changes applied before test

Patch application report for D3682 (id=12972)

Changes applied before test

Patch application report for D3682 (id=12973)

Changes applied before test

Patch application report for D3682 (id=12989)

Changes applied before test

Patch application report for D3682 (id=12990)

Changes applied before test

Revision Contents
Changeset List

Diff 12973

swh/scheduler/cli/task.py

swh/scheduler/tests/test_cli.py

cli.task: Migrate scheduler cli to latest storage change on iter_originsClosedPublicActions

Details

Diff Detail

Unit TestsFailedView All

Event Timeline

Patch application report for D3682 (id=12964)

Changes applied before test

Patch application report for D3682 (id=12965)

Changes applied before test

Patch application report for D3682 (id=12968)

Changes applied before test

Patch application report for D3682 (id=12972)

Changes applied before test

Patch application report for D3682 (id=12973)

Changes applied before test

Patch application report for D3682 (id=12989)

Changes applied before test

Patch application report for D3682 (id=12990)

Changes applied before test

Revision ContentsChangeset List

Diff 12973

swh/scheduler/cli/task.py

swh/scheduler/tests/test_cli.py

cli.task: Migrate scheduler cli to latest storage change on iter_origins
ClosedPublic
Actions

Unit TestsFailed
View All

Revision Contents
Changeset List