Page MenuHomeSoftware Heritage
Differential D3682

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

Authored by ardumont on Aug 1 2020, 10:04 AM.

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
swh-public-ci added a comment.Aug 1 2020, 10:07 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
291–293

why not use islice directly?

382

wrong order of args btw

vlorentz added a comment.EditedAug 1 2020, 10:09 AM

.

swh/scheduler/cli/task.py
372–376

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
291–293

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

382

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
372–376

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
372–376

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
372–376

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
372–376

yeah, indeed

ardumont added inline comments.Aug 1 2020, 10:39 AM
swh/scheduler/cli/task.py
372–376

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
ardumont updated this revision to Diff 12965.Aug 1 2020, 11:04 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)
swh-public-ci added a comment.Aug 1 2020, 11:07 AM

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
ardumont updated this revision to Diff 12968.EditedAug 1 2020, 11:27 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
swh-public-ci added a comment.Aug 1 2020, 11:31 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)
ardumont updated this revision to Diff 12972.Aug 1 2020, 7:03 PM

Fix tests appropriately

swh-public-ci added a comment.Aug 1 2020, 7:04 PM

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
ardumont updated this revision to Diff 12973.Aug 1 2020, 7:10 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

swh-public-ci added a comment.Aug 1 2020, 7:12 PM

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
vlorentz accepted this revision.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

This revision is now accepted and ready to land.Aug 3 2020, 9:13 AM
ardumont added a comment.Aug 3 2020, 9:48 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?

vlorentz added a comment.Aug 3 2020, 9:56 AM

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
ardumont added a comment.Aug 3 2020, 11:16 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)?

ardumont updated this revision to Diff 12989.Aug 3 2020, 12:11 PM

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
swh-public-ci added a comment.Aug 3 2020, 12:13 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
294

"Eventually" does not mean "éventuellement", but "finalement" ;)

Use "optionally" instead

ardumont updated this revision to Diff 12990.Aug 3 2020, 12:18 PM

Rework docstring sentences
(thx for the heads up)

swh-public-ci added a comment.Aug 3 2020, 12:20 PM

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.

Revision Contents
Changeset List

PathSize
swh/
scheduler/
cli/
44 lines
tests/
92 lines

Diff 12991

View Options

swh/scheduler/cli/task.py

Loading...
View Options

swh/scheduler/tests/test_cli.py

Loading...
About · Developer information · Legal