Details

Reviewers

Group Reviewers

Summary

This will ease scheduling of new add-forge-now requests, on:

staging: this will list a subset of disabled origins once
production: this will register recurring tasks (full, incremental if any) to list that new forge

So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
( --production | --staging \ # to list only enabled | disabled origins )
  register-lister \
    gitea \
    url=https://git.afpy.org/api/v1/

Test Plan

build happy

https://jenkins.softwareheritage.org/view/swh%20master%20(draft)/job/DDOC/job/build-on-diff/287/SWH_20sysadmin_20Documentation/

Diff Detail

Repository

rDSCH Scheduling utilities

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 33188
Build 52033: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 52032: arc lint + arc unit

Event Timeline

ardumont created this revision.Dec 7 2022, 5:00 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 7 2022, 5:00 PM

Build is green

Patch application report for D8940 (id=32213)

Rebasing onto 1c34e9837f...

Current branch diff-target is up to date.

Changes applied before test

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster completed remote builds in B33167: Diff 32213.Dec 7 2022, 5:05 PM

ardumont requested review of this revision.Dec 7 2022, 5:05 PM

So, I don't think we want to have to remember if there's full or incremental versions of the lister.

I would suggest only taking the forge type as argument (so, in your example, "gitea"), and pulling the list-{forge_type}-full / list-{forge_type}-incremental task types as needed. I guess these days we wouldn't even need both task types, just to pass incremental=true as argument to the task type?

For --staging, we just want to create a single oneshot full listing task. For --production, we want to create both tasks (if they exist) as recurring?

So, I don't think we want to have to remember if there's full or incremental versions
of the lister.

Yes, i did it that way without bothering with it to start small (small diff).

As not all listers have full/incremental implems (as per your own remarks too). I also
needed to go out soon, so i'd just cut to the chase ;) so we still could have something
to discuss upon.

I would suggest only taking the forge type as argument (so, in your example, "gitea"),
and pulling the list-{forge_type}-full / list-{forge_type}-incremental task types
as needed.

yes, sounds fine.

I guess these days we wouldn't even need both task types, just to pass
incremental=true as argument to the task type?

I'm not entirely sure all our listers respect that so i'd prefer checking if there is
the incremental/full tasks or not. That'd also simplify reading the logs, avoiding to
sort out whether it's got incremental argument to True or not.

For --staging, we just want to create a single oneshot full listing task.

Why bother with a full listing?

Since we now got the means to limit the listing, i'd use it. That's a faster feedback
loop and without stressing twice the upstream forge (one round for staging and another
for production at a relatively small intervals of time).

Maybe I need to configure the default to a more sensible defaults though. Like 3 pages
with 10 results (so that we can see that the pagination works too). Currently it's a tad
small (1 page of 15 or something).

For --production, we want to create both tasks (if they exist) as recurring?

Yes, if both tasks exist, let's schedule those as recurring.
Possibly with a time delta.

Like do a full listing now. And the next day, start the incremental one.

douardda added a subscriber: douardda.Dec 8 2022, 11:22 AM

douardda added inline comments.

swh/scheduler/cli/add_forge_now.py
33	I'm not very fond of the staging/production flag here. It does not really make for the scheduler as is and is not very explicit on what/why is should be used. Something like '--enable-origins/--disable-origins' (with the explanation similar to what's in the help message) would make more sense to me.

In D8940#232501, @ardumont wrote:

For --staging, we just want to create a single oneshot full listing task.

Why bother with a full listing?

Since we now got the means to limit the listing, i'd use it. That's a faster feedback
loop and without stressing twice the upstream forge (one round for staging and another
for production at a relatively small intervals of time).

Maybe I need to configure the default to a more sensible defaults though. Like 3 pages
with 10 results (so that we can see that the pagination works too). Currently it's a tad
small (1 page of 15 or something).

Sure, that's what I meant: for staging, schedule a oneshot full listing task, but with the limiting and "origin disabling" options ("full" as opposed to "incremental")

olasd added inline comments.Dec 8 2022, 11:31 AM

swh/scheduler/cli/add_forge_now.py
33	But that's really what it does: create the tasks with appropriate flags for staging, or for production. It doesn't just only change the enablement status of the origins.

ardumont added inline comments.Dec 8 2022, 11:35 AM

swh/scheduler/cli/add_forge_now.py
33	And i had implemented as you wanted david yesterday and got convinced to adapt to this (for the `swh scheduler add-forge-now schedule-first-visits` cli. So now, i'm just unifying on that use. Also, i'm now pondering whether those flags should go one level up... `swh scheduler add-forge-now --staging\|--production <subcommand>`

For --staging, we just want to create a single oneshot full listing task.

Why bother with a full listing?

Since we now got the means to limit the listing, i'd use it. That's a faster feedback
loop and without stressing twice the upstream forge (one round for staging and another
for production at a relatively small intervals of time).

Maybe I need to configure the default to a more sensible defaults though. Like 3 pages
with 10 results (so that we can see that the pagination works too). Currently it's a tad
small (1 page of 15 or something).

Sure, that's what I meant: for staging, schedule a oneshot full listing task, but with the limiting and "origin disabling" options ("full" as opposed to "incremental")

cool.

I missed the full/incremental issue. I'll adapt to that.

olasd added inline comments.Dec 8 2022, 11:38 AM

swh/scheduler/cli/add_forge_now.py
33	Yeah, the flag can probably move to the command group. We could call it a "preset" and have `--preset staging` / `--preset production` (and potentially `--preset something-else`, in the future), instead of a flag.

ardumont added inline comments.Dec 8 2022, 11:59 AM

swh/scheduler/cli/add_forge_now.py
33	why not --environment? (that's what i'm currently writing in my attempts to docstring-ify the commands now, that makes more sense to me than preset).

ardumont marked 3 inline comments as done.Dec 8 2022, 12:15 PM

Adapt somewhat according to review (let jenkins builds stuff) [1]

[1] I encounter issue with my dev env (pytest, tox, pre-commit, the all shebang).

Build has FAILED

Patch application report for D8940 (id=32226)

Could not rebase; Attempt merge onto 1c34e9837f...

Updating 1c34e98..b6dd69a
Fast-forward
 swh/scheduler/cli/add_forge_now.py            |  87 ++++++++++++--
 swh/scheduler/cli/origin.py                   |   2 +-
 swh/scheduler/cli/task.py                     | 167 +++-----------------------
 swh/scheduler/cli/utils.py                    | 157 +++++++++++++++++++++++-
 swh/scheduler/tests/test_cli_add_forge_now.py |  56 ++++++++-
 5 files changed, 304 insertions(+), 165 deletions(-)

Changes applied before test

commit b6dd69ae764a478ab296b257356dae3e48ced118
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Dec 8 12:15:27 2022 +0100

    cli.add_forge_now: Rework according to review
    
    commit will be amended

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster failed remote builds in B33181: Diff 32226!Dec 8 2022, 12:17 PM

Fix

Build has FAILED

Patch application report for D8940 (id=32227)

Could not rebase; Attempt merge onto 1c34e9837f...

Updating 1c34e98..37fd63e
Fast-forward
 swh/scheduler/cli/add_forge_now.py            |  87 ++++++++++++--
 swh/scheduler/cli/origin.py                   |   2 +-
 swh/scheduler/cli/task.py                     | 167 +++-----------------------
 swh/scheduler/cli/utils.py                    | 157 +++++++++++++++++++++++-
 swh/scheduler/tests/test_cli_add_forge_now.py |  56 ++++++++-
 5 files changed, 304 insertions(+), 165 deletions(-)

Changes applied before test

commit 37fd63e463d4d82dbe7c1277c9ebf18d2bf8c341
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Dec 8 12:15:27 2022 +0100

    cli.add_forge_now: Rework according to review
    
    commit will be amended

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster failed remote builds in B33182: Diff 32227!Dec 8 2022, 12:25 PM

Fix

Build has FAILED

Patch application report for D8940 (id=32228)

Could not rebase; Attempt merge onto 1c34e9837f...

Updating 1c34e98..183e506
Fast-forward
 swh/scheduler/cli/add_forge_now.py            |  87 ++++++++++++--
 swh/scheduler/cli/origin.py                   |   2 +-
 swh/scheduler/cli/task.py                     | 167 +++-----------------------
 swh/scheduler/cli/utils.py                    | 157 +++++++++++++++++++++++-
 swh/scheduler/tests/test_cli_add_forge_now.py |  58 ++++++++-
 5 files changed, 306 insertions(+), 165 deletions(-)

Changes applied before test

commit 183e506c20285e8c54a5f8aaa92b03b8b0c0b237
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Dec 8 12:15:27 2022 +0100

    cli.add_forge_now: Rework according to review
    
    commit will be amended

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster failed remote builds in B33183: Diff 32228!Dec 8 2022, 12:31 PM

cli.add_forge_now: Open register-lister with sensible defaults
cli.add_forge_now: Rework according to review

Build has FAILED

Patch application report for D8940 (id=32229)

Rebasing onto 1c34e9837f...

Current branch diff-target is up to date.

Changes applied before test

commit 77851263f7b5475e31670b827efcce7dd66695bb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Dec 8 12:15:27 2022 +0100

    cli.add_forge_now: Rework according to review
    
    commit will be amended

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster failed remote builds in B33184: Diff 32229!Dec 8 2022, 12:47 PM

Amend according to discussion and fix tests

Remains one issue with my venv which refuses to see that new cli... (that's annoying)

Build is green

Patch application report for D8940 (id=32232)

Rebasing onto 1c34e9837f...

Current branch diff-target is up to date.

Changes applied before test

commit da8fe7c92fa1c6c9cca3b91b2a0096ab70e5dd96
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Dec 8 12:15:27 2022 +0100

    cli.add_forge_now: Rework according to review
    
    commit will be amended

commit 63ee5c010024de47cb0e1ecd1cb2e63b68cce37d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This should ease the scheduling of a new forge:
    - on staging, this will list a subset of disabled origins
    - on production, this will register a recurring task to list a forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
  register-lister \
    list-gitea-full \
    ( --production | --staging \ # to list only enabled | disabled origins )
    url=https://git.afpy.org/api/v1/

```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster completed remote builds in B33187: Diff 32232.Dec 8 2022, 4:26 PM

Expose the new subcommand

ardumont edited the summary of this revision. (Show Details)Dec 8 2022, 5:00 PM

Build is green

Patch application report for D8940 (id=32233)

Rebasing onto 1c34e9837f...

First, rewinding head to replay your work on top of it...
Applying: cli.add_forge_now: Open `schedule-first-visits` with sensible defaults
Using index info to reconstruct a base tree...
M	swh/scheduler/cli/origin.py
M	swh/scheduler/cli/utils.py
M	swh/scheduler/tests/test_cli_origin.py
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: cli.add_forge_now: Open `register-lister` with sensible defaults

Changes applied before test

commit 18d414a089e74c9f901ab1394c95729200eadcf6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This will ease scheduling of new add-forge-now requests, on:
    - staging: this will list a subset of disabled origins once
    - production: this will register recurring tasks (full, incremental if any) to list that
      new forge
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
( --production | --staging \ # to list only enabled | disabled origins )
  register-lister \
    gitea \
    url=https://git.afpy.org/api/v1/
```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster completed remote builds in B33188: Diff 32233.Dec 8 2022, 5:00 PM

ardumont mentioned this in D8929: sysadm: Add a "how to process add-forge-now requests".Dec 8 2022, 5:20 PM

Adapt according to last irc discussion -> use --preset

olasd added inline comments.Dec 8 2022, 5:28 PM

swh/scheduler/cli/add_forge_now.py
50–55

Build is green

Patch application report for D8940 (id=32243)

Rebasing onto 1c34e9837f...

Current branch diff-target is up to date.

Changes applied before test

commit e71ce0ca797534611521d1d187a5beb0727785a7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Dec 7 16:57:32 2022 +0100

    cli.add_forge_now: Open `register-lister` with sensible defaults
    
    This will ease scheduling of new add-forge-now requests, on:
    - staging: this will list a subset of disabled origins once
    - production: this will register recurring tasks (full, incremental if any) to list that
      new forge
    
    This also unifies the previous subcommand schedule-first-visits with the --preset flag.
    
    So, the following would be enough to list appropriately in staging/production:

swh scheduler add-forge-now \
( --preset [production|staging] \ # to enable a pre-defined set of rules )
  register-lister \
    gitea \
    url=https://git.afpy.org/api/v1/
```

Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4674

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

Harbormaster completed remote builds in B33198: Diff 32243.Dec 8 2022, 5:28 PM

ardumont added inline comments.Dec 8 2022, 6:50 PM

swh/scheduler/cli/add_forge_now.py
50–55	thumbs up

ardumont edited the test plan for this revision. (Show Details)Dec 9 2022, 10:17 AM

I like it, thanks!

This revision is now accepted and ready to land.Dec 9 2022, 2:54 PM

Closed by 8e125f1

cli.add_forge_now: Open `register-lister` with sensible defaults
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D8940 (id=32213)

Changes applied before test

Patch application report for D8940 (id=32226)

Changes applied before test

Patch application report for D8940 (id=32227)

Changes applied before test

Patch application report for D8940 (id=32228)

Changes applied before test

Patch application report for D8940 (id=32229)

Changes applied before test

Patch application report for D8940 (id=32232)

Changes applied before test

Patch application report for D8940 (id=32233)

Changes applied before test

Patch application report for D8940 (id=32243)

Changes applied before test

Revision Contents
Changeset List

Diff 32233

requirements-test.txt

swh/scheduler/cli/init.py

swh/scheduler/cli/add_forge_now.py

swh/scheduler/cli/origin.py

swh/scheduler/cli/task.py

swh/scheduler/cli/test_cli_utils.py

swh/scheduler/cli/utils.py

swh/scheduler/tests/test_cli_add_forge_now.py

swh/scheduler/tests/test_cli_origin.py

cli.add_forge_now: Open `register-lister` with sensible defaultsClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D8940 (id=32213)

Changes applied before test

Patch application report for D8940 (id=32226)

Changes applied before test

Patch application report for D8940 (id=32227)

Changes applied before test

Patch application report for D8940 (id=32228)

Changes applied before test

Patch application report for D8940 (id=32229)

Changes applied before test

Patch application report for D8940 (id=32232)

Changes applied before test

Patch application report for D8940 (id=32233)

Changes applied before test

Patch application report for D8940 (id=32243)

Changes applied before test

Revision ContentsChangeset List

Diff 32233

requirements-test.txt

swh/scheduler/cli/__init__.py

swh/scheduler/cli/add_forge_now.py

swh/scheduler/cli/origin.py

swh/scheduler/cli/task.py

swh/scheduler/cli/test_cli_utils.py

swh/scheduler/cli/utils.py

swh/scheduler/tests/test_cli_add_forge_now.py

swh/scheduler/tests/test_cli_origin.py

cli.add_forge_now: Open `register-lister` with sensible defaults
ClosedPublic
Actions

Revision Contents
Changeset List

swh/scheduler/cli/init.py