Page MenuHomeSoftware Heritage
Differential D7713

crates: rework to take advantage of data returned by the crates lister
ClosedPublic
Actions

Authored by franckbret on Apr 29 2022, 10:14 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T4104: Ingest crates.io (Rust)
Commits
rDLDBASEa097a946c2f2: crates: rework to take advantage of data returned by the crates lister
Summary

The loader get 'artifacts' information through lister
'extra_loader_arguments'.

Related to T4104

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

franckbret created this revision.Apr 29 2022, 10:14 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 29 2022, 10:14 AM
swh-public-ci added a comment.Apr 29 2022, 10:17 AM

Build is green

Patch application report for D7713 (id=27897)

Rebasing onto 2e27f7c7a6...

Current branch diff-target is up to date.
Changes applied before test
commit 995e2b0fa8c0304bd671c473cfc8650427ce2308
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:02:20 2022 +0200

    crates: rework to take advantage of data returned by the crates lister
    
    The loader get 'artifacts' information through lister
    'extra_loader_arguments'.
    
    Related to T4104

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

Harbormaster completed remote builds in B28962: Diff 27897.Apr 29 2022, 10:17 AM
franckbret requested review of this revision.Apr 29 2022, 10:17 AM
franckbret updated this revision to Diff 27898.Apr 29 2022, 10:37 AM

Add 'Franck Bret' to contributors

ardumont added a subscriber: ardumont.Apr 29 2022, 10:40 AM

@franckbret

Arf, i reloaded the page (i was reviewing) but now lost the initial content of the diff.
You need to update back the diff with the range of commits you want to use.

arc diff HEAD~2 --update D7713  # assuming you are 2 commits ahead of what you want to diff.
swh-public-ci added a comment.Apr 29 2022, 10:41 AM

Build is green

Patch application report for D7713 (id=27898)

Could not rebase; Attempt merge onto 2e27f7c7a6...

Updating 2e27f7c..4ca8420
Fast-forward
 CONTRIBUTORS                                   |   1 +
 docs/package-loader-specifications.rst         |   2 +-
 swh/loader/package/crates/loader.py            |  57 ++++++-----
 swh/loader/package/crates/tasks.py             |   6 +-
 swh/loader/package/crates/tests/test_crates.py | 136 ++++++++++++++++++++-----
 swh/loader/package/crates/tests/test_tasks.py  |   5 +-
 6 files changed, 149 insertions(+), 58 deletions(-)
Changes applied before test
commit 4ca8420d2024a4c36cb3bf2d8985ae14789d238c
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:35:40 2022 +0200

    Add 'Franck Bret' to contributors

commit 995e2b0fa8c0304bd671c473cfc8650427ce2308
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:02:20 2022 +0200

    crates: rework to take advantage of data returned by the crates lister
    
    The loader get 'artifacts' information through lister
    'extra_loader_arguments'.
    
    Related to T4104

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

Harbormaster completed remote builds in B28963: Diff 27898.Apr 29 2022, 10:41 AM
franckbret updated this revision to Diff 27899.Apr 29 2022, 10:42 AM

Update back the diff

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

Build has FAILED

Patch application report for D7713 (id=27899)

Rebasing onto 2e27f7c7a6...

Current branch diff-target is up to date.
Changes applied before test
commit 4ca8420d2024a4c36cb3bf2d8985ae14789d238c
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:35:40 2022 +0200

    Add 'Franck Bret' to contributors

commit 995e2b0fa8c0304bd671c473cfc8650427ce2308
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:02:20 2022 +0200

    crates: rework to take advantage of data returned by the crates lister
    
    The loader get 'artifacts' information through lister
    'extra_loader_arguments'.
    
    Related to T4104

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

Harbormaster failed remote builds in B28964: Diff 27899!Apr 29 2022, 10:43 AM
swh-public-ci added a comment.Apr 29 2022, 10:49 AM

Build has FAILED

Patch application report for D7713 (id=27899)

Rebasing onto 2e27f7c7a6...

Current branch diff-target is up to date.
Changes applied before test
commit 4ca8420d2024a4c36cb3bf2d8985ae14789d238c
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:35:40 2022 +0200

    Add 'Franck Bret' to contributors

commit 995e2b0fa8c0304bd671c473cfc8650427ce2308
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:02:20 2022 +0200

    crates: rework to take advantage of data returned by the crates lister
    
    The loader get 'artifacts' information through lister
    'extra_loader_arguments'.
    
    Related to T4104

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

ardumont added inline comments.Apr 29 2022, 10:58 AM
swh/loader/package/crates/loader.py
239

You could make that dict of key artifact version, value artifact. ^

That'd simplify some stuff below.

all_versions = sorted(list(self.artifacts.keys()))
# artifact[version] for some value of version...
259

wondering whether that could work (although it's only use twice, one in the package loader and another with this ;)
so ¯\_(ツ)_/¯

269

with my previous suggestion.

294–298

with my previous suggestion.

ardumont added inline comments.Apr 29 2022, 11:09 AM
swh/loader/package/crates/loader.py
300–301

There is some inconsistency here now i think.
This ExtrincitPackageMetadata should be done for the artifact url.

The self.info method is using self.url which is now the origin url (and not the artifact url).
So i think it's missing some refactoring to pass along the url to actually use.

That will then avoid the "strange" filtering command for the crate version just one line after my comment.

ardumont added inline comments.Apr 29 2022, 11:13 AM
swh/loader/package/crates/loader.py
245–251
254–257

to be consistent with my concern about 'inconsistent behavior' below.

300–301

with my previous adaptations.

ardumont requested changes to this revision.Apr 29 2022, 11:25 AM

It's going the right way, thanks.

Still, I think there is some inconsistency that needs lifting.

See my suggestions inline.

(I did not check the tests yet)

This revision now requires changes to proceed.Apr 29 2022, 11:25 AM
franckbret marked 2 inline comments as done.Apr 29 2022, 3:56 PM
franckbret added inline comments.
swh/loader/package/crates/loader.py
239

Sure!

300–301

Not sure for this.. Maybe I miss something so I may be wrong.

The api endpoint is for one origin, it returns generic + information for each version.
Let's say that for crate 'x' we call https//crates.io/api/v1/crates/x
We just need to call it once, no matter if there is one or many versions.

Calling self.info(artifact['url']) will fail because artifact['url'] is the archive to download not an url suitable to make an api call to get extrinsic metadata.

We can sure call a more specific endpoint by adding version to the http api url (something like this https//crates.io/api/v1/crates/x/0.0.1) but we will do as many http api call as existing versions for this crate.

ardumont resigned from this revision.Apr 29 2022, 7:42 PM

I'll let other have the last word (i'm going afk for a bit ;)

ardumont removed reviewers: Reviewers, ardumont.Apr 29 2022, 7:42 PM
This revision now requires review to proceed.Apr 29 2022, 7:42 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 29 2022, 7:42 PM
ardumont added 1 blocking reviewer(s): Reviewers.Apr 29 2022, 7:42 PM
ardumont added inline comments.Apr 29 2022, 7:48 PM
swh/loader/package/crates/loader.py
300–301

The api endpoint is for one origin, it returns generic + information for each version.
Let's say that for crate 'x' we call https//crates.io/api/v1/crates/x
We just need to call it once, no matter if there is one or many versions.

ah yeah, thx, that makes sense.

ardumont added inline comments.Apr 29 2022, 7:50 PM
swh/loader/package/crates/loader.py
245–251

nevermind that.

254–257

nvm that too.

vlorentz accepted this revision.May 2 2022, 1:49 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/loader/package/crates/loader.py
212
This revision is now accepted and ready to land.May 2 2022, 1:49 PM
franckbret updated this revision to Diff 27971.May 4 2022, 10:07 AM
franckbret marked 3 inline comments as done.

Updating D7713: crates: rework to take advantage of data returned by the crates lister

Some small fixes after @ardumont and @vlorentz review.
Add myself to contributors.

swh-public-ci added a comment.May 4 2022, 10:10 AM

Build is green

Patch application report for D7713 (id=27971)

Rebasing onto c06305e78a...

Current branch diff-target is up to date.
Changes applied before test
commit a097a946c2f2716ff0410192754cf959e1df5ee0
Author: Franck Bret <franck.bret@octobus.net>
Date:   Fri Apr 29 10:02:20 2022 +0200

    crates: rework to take advantage of data returned by the crates lister
    
    The loader get 'artifacts' information through lister
    'extra_loader_arguments'.
    
    Related to T4104

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

Harbormaster completed remote builds in B29049: Diff 27971.May 4 2022, 10:10 AM
Closed by commit rDLDBASEa097a946c2f2: crates: rework to take advantage of data returned by the crates lister (authored by franckbret). · Explain WhyMay 4 2022, 10:31 AM
This revision was automatically updated to reflect the committed changes.
franckbret added a commit: rDLDBASEa097a946c2f2: crates: rework to take advantage of data returned by the crates lister.

Revision Contents
Changeset List

PathSize
1 line
docs/
2 lines
swh/
loader/
package/
crates/
59 lines
6 lines
tests/
127 lines
5 lines

Diff 27972

View Options

CONTRIBUTORS

Loading...
View Options

docs/package-loader-specifications.rst

Loading...
View Options

swh/loader/package/crates/loader.py

Loading...
View Options

swh/loader/package/crates/tasks.py

Loading...
View Options

swh/loader/package/crates/tests/test_crates.py

Loading...
View Options

swh/loader/package/crates/tests/test_tasks.py

Loading...
About · Developer information · Legal