Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T2998: Deploy lister next-gen in staging

Commits

rDLSae17b6b9a04a: Make stateless lister constructors compatible with credentials

Summary

In effect, it just allows to add credentials to cgit, cran and pypi listers.

This fixes instances of error [1]

[1] https://sentry.softwareheritage.org/share/issue/2c35a9f129cf4982a2dd003a232d507a/

Related to T2998

Test Plan

Tested on staging and it worked.

Diff Detail

Repository

rDLS Listers

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 18849
Build 29201: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 29200: arc lint + arc unit

Event Timeline

ardumont created this revision.Jan 28 2021, 1:34 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJan 28 2021, 1:34 PM

Build is green

Patch application report for D4961 (id=17702)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.

Changes applied before test

commit b6ec36a44e1c73a55079b157ece9379aef3aa2c4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jan 28 13:31:45 2021 +0100

    cgit: Make lister constructor compatible
    
    In effect, it just allows to add credentials even though those are not used. This fixes
    issue [1]
    
    [1] https://sentry.softwareheritage.org/share/issue/2c35a9f129cf4982a2dd003a232d507a/
    
    Related to T2998

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

Harbormaster completed remote builds in B18843: Diff 17702.Jan 28 2021, 1:38 PM

ardumont requested review of this revision.Jan 28 2021, 1:38 PM

LGTM. Other ported listers might have the same issue so I guess they should be fixed too.

Could you also update the celery tasks tests by mocking credentials configuration coming from configuration file ?

This revision is now accepted and ready to land.Jan 28 2021, 1:45 PM

Could you also update the celery tasks tests by mocking credentials configuration coming from configuration file ?

good idea, i'll check that.

Independently from this diff but still i think possibly related to stateless lister in general, you might be interested by [1]

[1] https://forge.softwareheritage.org/T2998#57500

ardumont edited the test plan for this revision. (Show Details)Jan 28 2021, 1:49 PM

In D4961#124922, @ardumont wrote:

Could you also update the celery tasks tests by mocking credentials configuration coming from configuration file ?

good idea, i'll check that.

Independently from this diff but still i think possibly related to stateless lister in general, you might be interested by [1]

[1] https://forge.softwareheritage.org/T2998#57500

load_from_envvar = mocker.patch("swh.lister.pattern.load_from_envvar")
load_from_envvar.return_value = {"credentials": {}}

Other ported listers might have the same issue so I guess they should be fixed too.

I had checked the phabricator and gitea one which are already ok btw.
I'll check if i missed others as well.

I'll check if i missed others as well.

yes, pypi and cran needs that patch as well.
I'll amend this diff.

Thanks for the heads up.

load_from_envvar = mocker.patch("swh.lister.pattern.load_from_envvar")
load_from_envvar.return_value = {"credentials": {}}

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

In D4961#124940, @ardumont wrote:

load_from_envvar = mocker.patch("swh.lister.pattern.load_from_envvar")
load_from_envvar.return_value = {"credentials": {}}

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

Ah right, I guess we should add the test in test_lister.py then. I will update D4962 then.

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

Ah right, I guess we should add the test in test_lister.py then. I will update D4962 then.

yes, indeed and that's what i'm currently doing ;)

(great news about launchpad ;)

In D4961#124944, @ardumont wrote:

Well, we can't really do that, we currently totally inhibit the lister.from_configfile call.

Ah right, I guess we should add the test in test_lister.py then. I will update D4962 then.

yes, indeed and that's what i'm currently doing ;)

(great news about launchpad ;)

FYI, I did it like this for launchpad.

def test_lister_from_configfile(swh_scheduler_config, mocker):
    load_from_envvar = mocker.patch("swh.lister.pattern.load_from_envvar")
    load_from_envvar.return_value = {
        "scheduler": {"cls": "local", **swh_scheduler_config},
        "credentials": {},
    }
    lister = LaunchpadLister.from_configfile()
    assert lister.scheduler is not None
    assert lister.credentials is not None

Update cgit/cran/pypi lister to allow credentials to be passed along during instantiation

Fix typos

Build has FAILED

Patch application report for D4961 (id=17705)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.

Changes applied before test

commit f1d69d969f24fa643b3419bb6ef513beff93d3b9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jan 28 13:31:45 2021 +0100

    cgit: Make lister constructor compatible
    
    In effect, it just allows to add credentials even though those are not used. This fixes
    issue [1]
    
    [1] https://sentry.softwareheritage.org/share/issue/2c35a9f129cf4982a2dd003a232d507a/
    
    Related to T2998

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

Harbormaster failed remote builds in B18846: Diff 17705!Jan 28 2021, 2:32 PM

Build is green

Patch application report for D4961 (id=17706)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.

Changes applied before test

commit 3e25c6197dc3138534d98b95b6837c076f300caf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jan 28 13:31:45 2021 +0100

    cgit: Make lister constructor compatible
    
    In effect, it just allows to add credentials even though those are not used. This fixes
    issue [1]
    
    [1] https://sentry.softwareheritage.org/share/issue/2c35a9f129cf4982a2dd003a232d507a/
    
    Related to T2998

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

Harbormaster completed remote builds in B18847: Diff 17706.Jan 28 2021, 2:36 PM

Add *Lister.from_configfile test for stateless listers (cgit, cran, pypi)

ardumont retitled this revision from cgit: Make lister constructor compatible to Make stateless lister constructors compatible with credentials.Jan 28 2021, 2:44 PM

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

Build is green

Patch application report for D4961 (id=17708)

Rebasing onto 72be074a79...

Current branch diff-target is up to date.

Changes applied before test

commit ae17b6b9a04a473eb5957572ea2063f685bcf194
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jan 28 13:31:45 2021 +0100

    Make stateless lister constructors compatible with credentials
    
    In effect, it just allows to add credentials to cgit, cran and pypi listers.
    
    This fixes instances of error [1]
    
    [1] https://sentry.softwareheritage.org/share/issue/2c35a9f129cf4982a2dd003a232d507a/
    
    Related to T2998

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

Harbormaster completed remote builds in B18849: Diff 17708.Jan 28 2021, 2:47 PM

Closed by commit rDLSae17b6b9a04a: Make stateless lister constructors compatible with credentials (authored by ardumont). · Explain WhyJan 28 2021, 2:50 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDLSae17b6b9a04a: Make stateless lister constructors compatible with credentials.

Make stateless lister constructors compatible with credentials
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D4961 (id=17702)

Changes applied before test

Patch application report for D4961 (id=17705)

Changes applied before test

Patch application report for D4961 (id=17706)

Changes applied before test

Patch application report for D4961 (id=17708)

Changes applied before test

Revision Contents
Changeset List

Diff 17708

swh/lister/cgit/lister.py

swh/lister/cgit/tests/test_lister.py

swh/lister/cgit/tests/test_tasks.py

swh/lister/cran/lister.py

swh/lister/cran/tests/test_lister.py

swh/lister/pypi/lister.py

swh/lister/pypi/tests/test_lister.py

Make stateless lister constructors compatible with credentialsClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D4961 (id=17702)

Changes applied before test

Patch application report for D4961 (id=17705)

Changes applied before test

Patch application report for D4961 (id=17706)

Changes applied before test

Patch application report for D4961 (id=17708)

Changes applied before test

Revision ContentsChangeset List

Diff 17708

swh/lister/cgit/lister.py

swh/lister/cgit/tests/test_lister.py

swh/lister/cgit/tests/test_tasks.py

swh/lister/cran/lister.py

swh/lister/cran/tests/test_lister.py

swh/lister/pypi/lister.py

swh/lister/pypi/tests/test_lister.py

Make stateless lister constructors compatible with credentials
ClosedPublic
Actions

Revision Contents
Changeset List