Page MenuHomeSoftware Heritage
Differential D7329

Non-incremental Golang module lister
ClosedPublic
Actions

Authored by Alphare on Mar 10 2022, 12:18 AM.

Details

Reviewers
vlorentz
anlambert
Group Reviewers
Reviewers
Commits
rDLS60405e78aefd: Add non-incremental Golang modules lister

Diff Detail

Repository
rDLS Listers
Branch
golang-D7329
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31178
Build 48769: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 48768: arc lint + arc unit

Event Timeline

Alphare created this revision.Mar 10 2022, 12:18 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 10 2022, 12:18 AM
swh-public-ci added a comment.Mar 10 2022, 12:19 AM

Build has FAILED

Patch application report for D7329 (id=26502)

Rebasing onto 2568ecc7c2...

Current branch diff-target is up to date.
Changes applied before test
commit 59876002ce11f91cadedc64ba7fd6fb8b49b5e70
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    WIP basic golang loader

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

Harbormaster failed remote builds in B27387: Diff 26502!Mar 10 2022, 12:19 AM
Alphare updated this revision to Diff 26503.Mar 10 2022, 10:43 AM

First reviewable version

swh-public-ci added a comment.Mar 10 2022, 10:43 AM

Build has FAILED

Patch application report for D7329 (id=26503)

Rebasing onto 2568ecc7c2...

Current branch diff-target is up to date.
Changes applied before test
commit 9d601d965910540692d9ac629ce6228975c29365
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster failed remote builds in B27388: Diff 26503!Mar 10 2022, 10:43 AM
Alphare updated this revision to Diff 26504.Mar 10 2022, 10:48 AM

Fix requirements

swh-public-ci added a comment.Mar 10 2022, 10:49 AM

Build has FAILED

Patch application report for D7329 (id=26504)

Rebasing onto 2568ecc7c2...

Current branch diff-target is up to date.
Changes applied before test
commit 75867e86b129c05fc0df442da94fb6c52d4ca256
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster failed remote builds in B27389: Diff 26504!Mar 10 2022, 10:49 AM
Alphare updated this revision to Diff 26505.Mar 10 2022, 10:59 AM

Fix requirements

swh-public-ci added a comment.Mar 10 2022, 11:02 AM

Build is green

Patch application report for D7329 (id=26505)

Rebasing onto 2568ecc7c2...

Current branch diff-target is up to date.
Changes applied before test
commit 0bfe53cc52374b510441ce78d6aeae7b6fd08238
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B27390: Diff 26505.Mar 10 2022, 11:02 AM
Alphare updated this revision to Diff 26506.Mar 10 2022, 11:15 AM

Reuse URL

swh-public-ci added a comment.Mar 10 2022, 11:18 AM

Build is green

Patch application report for D7329 (id=26506)

Rebasing onto 2568ecc7c2...

Current branch diff-target is up to date.
Changes applied before test
commit 736a2f5ea923ac5e75412a15b314fed1df069f92
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B27391: Diff 26506.Mar 10 2022, 11:18 AM
Alphare published this revision for review.Mar 10 2022, 4:15 PM
Alphare retitled this revision from WIP basic golang loader to Non-incremental Golang module lister.
Alphare added a project: Lister.
vlorentz added a subscriber: vlorentz.Mar 10 2022, 4:23 PM

Looks good, thanks.

swh/lister/golang/lister.py
11

Use iso8601 instead, it's smaller and we already depend on it. RFC 3339 is a subset of ISO 8601, so it should be fine.

24

(we could use a TypedDict to be even more specific, but I don't think it's worth it)

84

Please add an assertion that times are indeed UTC

ardumont added a subscriber: ardumont.Mar 10 2022, 5:43 PM
ardumont added inline comments.
swh/lister/golang/lister.py
11

we also moved away from dateutil parser which is way too lenient (and answers suprising results)

Alphare updated this revision to Diff 26536.Mar 11 2022, 2:51 PM

Remove use of dateutil and add small fixes

Alphare marked 4 inline comments as done.Mar 11 2022, 2:52 PM
swh-public-ci added a comment.Mar 11 2022, 2:55 PM

Build is green

Patch application report for D7329 (id=26536)

Rebasing onto fd03941c5f...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit 1f959174fb44a863a08b1c72a63ac5a7c4c8f093
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B27419: Diff 26536.Mar 11 2022, 2:55 PM
vlorentz added a comment.Mar 14 2022, 3:13 PM

LGTM

Alphare added a comment.Mar 14 2022, 5:51 PM
In D7329#190995, @vlorentz wrote:

LGTM

Thanks. Should I add a tests_tasks.py? I'm not 100% sure of their usefulness but they're in all other packages.

ardumont added a comment.Mar 14 2022, 6:59 PM

LGTM

Thanks. Should I add a tests_tasks.py? I'm not 100% sure of their usefulness but they're in all other packages.

They kinda are since we are making sure the celery registration happens iirc.
That avoids surprises when actually deploying in our infra.

Alphare updated this revision to Diff 26580.Mar 16 2022, 10:17 AM

Add test_tasks.py

swh-public-ci added a comment.Mar 16 2022, 10:20 AM

Build is green

Patch application report for D7329 (id=26580)

Rebasing onto fd03941c5f...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit 01f14f2257b3195362c1ee31fe3cc7f81bb61d73
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B27460: Diff 26580.Mar 16 2022, 10:20 AM
ardumont added a comment.EditedMar 16 2022, 4:50 PM

@Alphare lgtm

some cosmetic changes suggested inline.

Can you please:

  • update the README to reference that new lister?
  • make this run within docker (if not already done) or something to make sure it's working as expected [1]

[1] or some other stack, I recall you don't use docker or something. No need to make it
run fully, just making sure some pages of results actually renders listed origins
records in the listed_origins scheduler table.

Thanks in advance.

swh/lister/golang/lister.py
79–80

f-strings all the way ;)

90

same about f-string?

ardumont mentioned this in T4124: Golang support.Apr 1 2022, 6:05 PM
Alphare updated this revision to Diff 29902.Aug 22 2022, 3:59 PM
Alphare marked 2 inline comments as done.

Update README, sort setup.py and use f-strings

swh-public-ci added a comment.Aug 22 2022, 4:04 PM

Build is green

Patch application report for D7329 (id=29902)

Rebasing onto dde7865ac4...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit 8016c6418226cd674846d80ae9362b99277bd4e1
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B30955: Diff 29902.Aug 22 2022, 4:04 PM
Alphare added a child revision: D8298: Add incremental function to Golang Lister.Aug 23 2022, 5:11 PM
vlorentz added inline comments.Aug 24 2022, 8:41 AM
swh/lister/golang/lister.py
76–78

Please add a docstring to explain how since is used and the second returned value (I found it confusing while reviewing D8298)

Alphare updated this revision to Diff 29969.Aug 24 2022, 11:09 AM

Add docstring

Alphare marked an inline comment as done.Aug 24 2022, 11:09 AM
swh-public-ci added a comment.Aug 24 2022, 11:13 AM

Build is green

Patch application report for D7329 (id=29969)

Rebasing onto 4b511b4181...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit 55749848c3f2bcafe5428f02439843a565743dca
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B31019: Diff 29969.Aug 24 2022, 11:13 AM
vlorentz accepted this revision.Aug 24 2022, 4:51 PM
This revision is now accepted and ready to land.Aug 24 2022, 4:51 PM
Alphare updated this revision to Diff 29999.Aug 24 2022, 6:36 PM

List per package, not per version, prefix with proxy URL

This does not group the ListedOrigin per package yet. I still feel like this
is not necessary since they are upserted and batched, so the performance
penalty should be negligible. Either way I don't have time to write that today,
might as well post a correct implementation in the mean time.

swh-public-ci added a comment.Aug 24 2022, 6:40 PM

Build is green

Patch application report for D7329 (id=29999)

Rebasing onto 4b511b4181...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit 8d64c912a24ad65175fd0a814950dd68a09c3ca8
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org and lists origins to be loaded using
    the Golang Module Proxy Protocol. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B31048: Diff 29999.Aug 24 2022, 6:40 PM
Alphare updated this revision to Diff 30019.Aug 25 2022, 11:58 AM

Use pkg.go.dev instead of the proxy

swh-public-ci added a comment.Aug 25 2022, 12:04 PM

Build is green

Patch application report for D7329 (id=30019)

Rebasing onto ce72969de5...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit a0755babd8c609197f89b8689e9c41bdde9faafb
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B31068: Diff 30019.Aug 25 2022, 12:04 PM
anlambert requested changes to this revision.Aug 25 2022, 4:30 PM
anlambert added a subscriber: anlambert.

I found an issue when testing the lister in our docker environment, see inline comment.

swh/lister/golang/lister.py
83–106

I gave a shot to the lister in our docker environment and could succesfully list all go packages,
however the lister never terminates as this code generates an infinite loop when all packages
got listed.

Indeed the latest listed package timestamp will be reused over and and over as
the since parameter returns packages whose timestamp are greater or equal to the date
provided as parameter.

I think adding another termination condition by saving the last since value used in
previous HTTP request and comparing it with the one of the request to be sent
should do the trick to prevent the infinite requests loop: if both values are equal
you can return an empty page and terminates the listing.

This revision now requires changes to proceed.Aug 25 2022, 4:30 PM
Alphare updated this revision to Diff 30083.Aug 29 2022, 12:20 PM

Fix infinite loop

swh-public-ci added a comment.Aug 29 2022, 12:25 PM

Build is green

Patch application report for D7329 (id=30083)

Rebasing onto 5410b6e3f3...

First, rewinding head to replay your work on top of it...
Applying: Add non-incremental Golang modules lister
Changes applied before test
commit cf17398bc64e28e9eaa6182ab574507c707b041a
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B31128: Diff 30083.Aug 29 2022, 12:25 PM
anlambert accepted this revision.Aug 30 2022, 12:30 PM

Looks good to me, thanks !

This revision is now accepted and ready to land.Aug 30 2022, 12:30 PM
Alphare updated this revision to Diff 30135.Aug 30 2022, 2:33 PM

Rebase

This revision was landed with ongoing or failed builds.Aug 30 2022, 2:35 PM
Closed by commit rDLS60405e78aefd: Add non-incremental Golang modules lister (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.
Alphare added a commit: rDLS60405e78aefd: Add non-incremental Golang modules lister.
swh-public-ci added a comment.Aug 30 2022, 2:39 PM

Build is green

Patch application report for D7329 (id=30135)

Rebasing onto 0acf5b0f4f...

Current branch diff-target is up to date.
Changes applied before test
commit 60405e78aefde8566ead1c9d2901ab64b689129d
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Wed Mar 9 22:35:40 2022 +0100

    Add non-incremental Golang modules lister
    
    This uses https://index.golang.org. An associated loader will be sent in
    the near future, as well as an incremental version of this lister.
    
    [1] https://go.dev/ref/mod#goproxy-protocol

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

Harbormaster completed remote builds in B31178: Diff 30135.Aug 30 2022, 2:39 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
3 lines--
1 line--
swh/
lister/
golang/
12 lines100%
Loading...
145 lines98%
Loading...
18 lines100%
Loading...
tests/
81%
Loading...
data/
5 lines--
4 lines--
10 lines--
90 lines100%
Loading...
32 lines100%
Loading...

Diff 30135

View Options

README.md

Loading...
View Options

setup.py

Loading...
View Options

swh/lister/golang/__init__.py

Loading...
View Options

swh/lister/golang/lister.py

Loading...
View Options

swh/lister/golang/tasks.py

Loading...
View Options

swh/lister/golang/tests/__init__.py

Loading...
View Options

swh/lister/golang/tests/data/page-1.txt

Loading...
View Options

swh/lister/golang/tests/data/page-2.txt

Loading...
View Options

swh/lister/golang/tests/data/page-3.txt

Loading...
View Options

swh/lister/golang/tests/test_lister.py

Loading...
View Options

swh/lister/golang/tests/test_tasks.py

Loading...
About · Developer information · Legal