Page MenuHomeSoftware Heritage
Differential D6313

gnu: Respect the pattern docstring about extra state initialization
ClosedPublic
Actions

Authored by ardumont on Sep 21 2021, 10:27 AM.

Details

Reviewers
anlambert
olasd
Group Reviewers
Reviewers
Commits
rDLS5ab6b0040806: gnu: Respect the pattern docstring about state initialization
Summary

Any extra state initialization (outside the scheduler scope) is to happen in the
get_pages method.

Following the discussion in D6310, the gnu lister happens to do that step wrong. So this
fixes it.

Test Plan

tox

Diff Detail

Repository
rDLS Listers
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.Sep 21 2021, 10:27 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 21 2021, 10:27 AM
ardumont mentioned this in D6310: opam: Move the state initialization into the get_pages method.Sep 21 2021, 10:29 AM
swh-public-ci added a comment.Sep 21 2021, 10:29 AM

Build is green

Patch application report for D6313 (id=22911)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..850805b
Fast-forward
 swh/lister/gnu/lister.py             | 10 +++++--
 swh/lister/opam/lister.py            | 18 ++++++++----
 swh/lister/opam/tests/test_lister.py | 53 ++++++++++++++++++++++++++++++------
 swh/lister/pattern.py                | 13 +++++++++
 4 files changed, 78 insertions(+), 16 deletions(-)
Changes applied before test
commit 850805b1ea6031d5182f02c563bcb7b45ec0bffb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 21 10:25:31 2021 +0200

    gnu: Respect the pattern docstring about state initialization
    
    Any extra state initialization (outside the scheduler scope) is to happen in the
    get_pages method.

commit 7996d102787950433eaee7ad986c5ddcdf0dcbc0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 0feddfaced6df52b06a145c1e9c905becf61b4d3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit 2d0099bf7206347298e38603130445e79a9bf1dd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Separate side-effects call from the constructor
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed should be now
    built within the `swh.lister.patter.Lister.prepare` method.
    
    This also fixes the current test that actually bootstrap a real opam local "clone".
    
    Related to T3590

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

Harbormaster completed remote builds in B23839: Diff 22911.Sep 21 2021, 10:29 AM
ardumont requested review of this revision.Sep 21 2021, 10:29 AM
ardumont updated this revision to Diff 22915.Sep 21 2021, 10:37 AM

Rebase

swh-public-ci added a comment.Sep 21 2021, 10:40 AM

Build is green

Patch application report for D6313 (id=22915)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..4ff277f
Fast-forward
 swh/lister/gnu/lister.py             | 10 +++++--
 swh/lister/opam/lister.py            | 20 ++++++++------
 swh/lister/opam/tests/test_lister.py | 53 ++++++++++++++++++++++++++++++------
 swh/lister/pattern.py                | 13 +++++++++
 4 files changed, 78 insertions(+), 18 deletions(-)
Changes applied before test
commit 4ff277f42776b32f8adc17513579674126e9fa2d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 21 10:25:31 2021 +0200

    gnu: Respect the pattern docstring about state initialization
    
    Any extra state initialization (outside the scheduler scope) is to happen in the
    get_pages method.

commit be96c385c24612ee265df42af71e3da09ca99393
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 34880c73a2154a24a6fd3e0f8b707b951f810ec8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit 0af407999ab4932e4dccf51897e0402d748a3f03
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Harbormaster completed remote builds in B23843: Diff 22915.Sep 21 2021, 10:40 AM
ardumont updated this revision to Diff 22919.Sep 21 2021, 10:49 AM

Rebase

swh-public-ci added a comment.Sep 21 2021, 10:52 AM

Build is green

Patch application report for D6313 (id=22919)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..fddba17
Fast-forward
 swh/lister/gnu/lister.py             | 10 +++++--
 swh/lister/opam/lister.py            | 20 +++++++------
 swh/lister/opam/tests/test_lister.py | 54 ++++++++++++++++++++++++++++++------
 3 files changed, 66 insertions(+), 18 deletions(-)
Changes applied before test
commit fddba1794b7e6819c8ea3842f3eaea21d661b9f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 21 10:25:31 2021 +0200

    gnu: Respect the pattern docstring about state initialization
    
    Any extra state initialization (outside the scheduler scope) is to happen in the
    get_pages method.

commit 50700fd47379c05b440ad6e97a905ec265a08b8d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 2398ca85c8c4685465945c305a67279a67f7ba55
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit c160376b17175bd8fa73adddf2e14b8dcfad947b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Harbormaster completed remote builds in B23847: Diff 22919.Sep 21 2021, 10:52 AM
anlambert accepted this revision.Sep 21 2021, 10:55 AM
anlambert added a subscriber: anlambert.

Looks good to me.

This revision is now accepted and ready to land.Sep 21 2021, 10:55 AM
olasd accepted this revision.Sep 21 2021, 10:56 AM
olasd added a subscriber: olasd.

Thank you!

swh/lister/gnu/lister.py
48

isn't that a tad redundant with the previous line?

ardumont updated this revision to Diff 22920.Sep 21 2021, 10:58 AM

Drop unneeded assertion

swh/lister/gnu/lister.py
48

yes, removing.

swh-public-ci added a comment.Sep 21 2021, 11:01 AM

Build is green

Patch application report for D6313 (id=22920)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..771ab44
Fast-forward
 swh/lister/gnu/lister.py             |  9 ++++--
 swh/lister/opam/lister.py            | 20 +++++++------
 swh/lister/opam/tests/test_lister.py | 54 ++++++++++++++++++++++++++++++------
 3 files changed, 65 insertions(+), 18 deletions(-)
Changes applied before test
commit 771ab445a3a03d05e1ae12f501e0df1b756aa767
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 21 10:25:31 2021 +0200

    gnu: Respect the pattern docstring about state initialization
    
    Any extra state initialization (outside the scheduler scope) is to happen in the
    get_pages method.

commit 50700fd47379c05b440ad6e97a905ec265a08b8d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 2398ca85c8c4685465945c305a67279a67f7ba55
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit c160376b17175bd8fa73adddf2e14b8dcfad947b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Harbormaster completed remote builds in B23848: Diff 22920.Sep 21 2021, 11:01 AM
ardumont updated this revision to Diff 22925.Sep 21 2021, 11:19 AM

Rebase

swh-public-ci added a comment.Sep 21 2021, 11:22 AM

Build is green

Patch application report for D6313 (id=22925)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..5ab6b00
Fast-forward
 swh/lister/gnu/lister.py             |  9 ++++--
 swh/lister/opam/lister.py            | 20 +++++++------
 swh/lister/opam/tests/test_lister.py | 54 ++++++++++++++++++++++++++++++------
 3 files changed, 65 insertions(+), 18 deletions(-)
Changes applied before test
commit 5ab6b0040806844762e6ae229c95555564a6b2ea
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Sep 21 10:25:31 2021 +0200

    gnu: Respect the pattern docstring about state initialization
    
    Any extra state initialization (outside the scheduler scope) is to happen in the
    get_pages method.

commit 332ed8e54309985e35acb1e7023a7a2ca6e9e88c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit ff5e86ff489eb1d3c504fe6f0a3392db4d8b6c19
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit b69b0b7fd6745cd579acf2e5544c3810217bb542
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Harbormaster completed remote builds in B23854: Diff 22925.Sep 21 2021, 11:22 AM
Closed by commit rDLS5ab6b0040806: gnu: Respect the pattern docstring about state initialization (authored by ardumont). · Explain WhySep 21 2021, 11:23 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLS5ab6b0040806: gnu: Respect the pattern docstring about state initialization.

Revision Contents
Changeset List

PathSize
swh/
lister/
gnu/
9 lines

Diff 22929

View Options

swh/lister/gnu/lister.py

Loading...
About · Developer information · Legal