Page MenuHomeSoftware Heritage

Non-incremental Golang module lister
ClosedPublic

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

Diff Detail

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

Event Timeline

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

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

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

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.

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.

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.

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 inline comments.
swh/lister/golang/lister.py
11

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

Remove use of dateutil and add small fixes

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.

LGTM

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

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.

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.

@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?

Alphare marked 2 inline comments as done.

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

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.

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)

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.

This revision is now accepted and ready to land.Aug 24 2022, 4:51 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.

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.

Use pkg.go.dev instead of the proxy

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.

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

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.

Looks good to me, thanks !

This revision is now accepted and ready to land.Aug 30 2022, 12:30 PM
This revision was landed with ongoing or failed builds.Aug 30 2022, 2:35 PM
This revision was automatically updated to reflect the committed changes.

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.