Page MenuHomeSoftware Heritage
Differential D8747

conda: Yield listed origins after all artifacts in a page are processed
ClosedPublic
Actions

Authored by anlambert on Oct 19 2022, 4:08 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDLS4f6b3f3f09b8: conda: Yield listed origins after all artifacts in a page are processed
Summary

swh-scheduler will deduplicate listed origins according to their URL
and visit type but not according to their extra loader arguments.

Previously, listed origins were yielded after each processed artifact
in a page so we could lose some package version info due to the
deduplication process.

So ensure to yield listed origins once all artifacts in a page have
been processed.

Diff Detail

Repository
rDLS Listers
Branch
conda-yield-origins-after-page-processing
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 32481
Build 50864: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 50863: arc lint + arc unit

Event Timeline

anlambert created this revision.Oct 19 2022, 4:08 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 19 2022, 4:08 PM
swh-public-ci added a comment.Oct 19 2022, 4:13 PM

Build is green

Patch application report for D8747 (id=31527)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit f001bc1bc1695af7d21cd8fad33d00ded51ccfcd
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Oct 19 15:59:25 2022 +0200

    conda: Yield listed origins after all artifacts in a page processed
    
    swh-scheduler will deduplicate listed origins according to their URL
    and visit type but not according to their extra loader arguments.
    
    Previously, listed origins were yielded after each processed artifact
    in a page so we could lose some package version info due to the
    deduplication process.
    
    So ensure to yield listed origins once all artifacts in a page have
    been processed.

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

Harbormaster completed remote builds in B32481: Diff 31527.Oct 19 2022, 4:13 PM
anlambert requested review of this revision.Oct 19 2022, 4:13 PM
douardda added a subscriber: douardda.Oct 20 2022, 10:44 AM

I believe the commit title lacks a verb, doesn't it? ("conda: Yield listed origins after all artifacts in a page are processed" or something similar?)

douardda added a comment.Oct 20 2022, 10:46 AM

shouldn't this fix come with a test of some sort?

anlambert retitled this revision from conda: Yield listed origins after all artifacts in a page processed to conda: Yield listed origins after all artifacts in a page are processed.Oct 20 2022, 3:39 PM
anlambert updated this revision to Diff 31531.Oct 20 2022, 3:40 PM

Add missing test

swh-public-ci added a comment.Oct 20 2022, 3:45 PM

Build is green

Patch application report for D8747 (id=31531)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit fc22f7539a7fda5294ae8135d7c08d86b4ad0870
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Oct 19 15:59:25 2022 +0200

    conda: Yield listed origins after all artifacts in a page are processed
    
    swh-scheduler will deduplicate listed origins according to their URL
    and visit type but not according to their extra loader arguments.
    
    Previously, listed origins were yielded after each processed artifact
    in a page so we could lose some package version info due to the
    deduplication process.
    
    So ensure to yield listed origins once all artifacts in a page have
    been processed.

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

Harbormaster completed remote builds in B32485: Diff 31531.Oct 20 2022, 3:45 PM
vlorentz requested changes to this revision.Oct 24 2022, 10:40 AM
vlorentz added a subscriber: vlorentz.

Could you add docstrings to the test? I don't understand what they are expected to test

swh/lister/conda/lister.py
119
121–123
swh/lister/conda/tests/test_lister.py
116 ↗(On Diff #31531)

Please compare the lists contents; the length alone is not very robust, and doesn't give meaningful messages on failure

This revision now requires changes to proceed.Oct 24 2022, 10:40 AM
anlambert added inline comments.Oct 24 2022, 1:33 PM
swh/lister/conda/lister.py
119

Nide, I did not know that keyword argument existed for max.

swh/lister/conda/tests/test_lister.py
116 ↗(On Diff #31531)

Those are not the same type in the lists so I cannot compare them and the purpose of that test is to check the number of sent origins to the scheduler is not greater that the number of expected origins. The checks of the data sent to scheduler are done in previous tests.

anlambert added inline comments.Oct 24 2022, 1:36 PM
swh/lister/conda/tests/test_lister.py
116 ↗(On Diff #31531)

I will modify the test to check for origin URLs instead.

anlambert updated this revision to Diff 31568.Oct 24 2022, 1:40 PM

Rebase and address @vlorentz comments

swh-public-ci added a comment.Oct 24 2022, 1:45 PM

Build is green

Patch application report for D8747 (id=31568)

Rebasing onto 8a82bbf95f...

Current branch diff-target is up to date.
Changes applied before test
commit 018fc641bfb2c578be762f1867c33101687bfc03
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Oct 19 15:59:25 2022 +0200

    conda: Yield listed origins after all artifacts in a page are processed
    
    swh-scheduler will deduplicate listed origins according to their URL
    and visit type but not according to their extra loader arguments.
    
    Previously, listed origins were yielded after each processed artifact
    in a page so we could lose some package version info due to the
    deduplication process.
    
    So ensure to yield listed origins once all artifacts in a page have
    been processed.

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

Harbormaster completed remote builds in B32522: Diff 31568.Oct 24 2022, 1:45 PM
vlorentz accepted this revision.Oct 24 2022, 6:26 PM

perfect, thanks

This revision is now accepted and ready to land.Oct 24 2022, 6:26 PM
anlambert updated this revision to Diff 31592.Oct 25 2022, 10:52 AM

Rebase

swh-public-ci added a comment.Oct 25 2022, 10:57 AM

Build is green

Patch application report for D8747 (id=31592)

Rebasing onto 31eb5f637f...

Current branch diff-target is up to date.
Changes applied before test
commit 4f6b3f3f09b8a8ab8a5e3679b3a8b936dbd21640
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Oct 19 15:59:25 2022 +0200

    conda: Yield listed origins after all artifacts in a page are processed
    
    swh-scheduler will deduplicate listed origins according to their URL
    and visit type but not according to their extra loader arguments.
    
    Previously, listed origins were yielded after each processed artifact
    in a page so we could lose some package version info due to the
    deduplication process.
    
    So ensure to yield listed origins once all artifacts in a page have
    been processed.

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

Harbormaster completed remote builds in B32546: Diff 31592.Oct 25 2022, 10:57 AM
Closed by commit rDLS4f6b3f3f09b8: conda: Yield listed origins after all artifacts in a page are processed (authored by anlambert). · Explain WhyOct 25 2022, 10:58 AM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDLS4f6b3f3f09b8: conda: Yield listed origins after all artifacts in a page are processed.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
lister/
conda/
12 lines100%
Loading...

Diff 31527

View Options

swh/lister/conda/lister.py

Loading...
About · Developer information · Legal