Paths

Table of Contentst

npm: Explicitely retrieve the revision date from extrinsic metadata
ClosedPublic
Actions

Authored by ardumont on Nov 22 2019, 4:18 PM.

Details

Reviewers

douardda
anlambert
olasd

Group Reviewers

Reviewers

Commits

rDLDBASE105e76d79a00: npm: Explicitely retrieve the revision date from extrinsic metadata

Summary

No date is available in intrinsic metadata, so we retrieve it from the API
metadata, using the version number that the API claims this package has.

This fixes the current loading which fails when this scenario occurs.

Note: This matches what was done in the prior loader-npm version [1]

[1] https://forge.softwareheritage.org/source/swh-loader-npm/browse/master/swh/loader/npm/client.py$84-91

Test Plan

swh-docker-dev:

from swh.loader.package.npm import NpmLoader;
l = NpmLoader(package_url='https://www.npmjs.com/package/@aller/shared', package_name='@aller/shared', package_metadata_url='https://replicate.npmjs.com/%40aller%2Fshared');
l.load()

worker0@staging

Without patch, explodes mid-air with KeyError on version mismatch.
With patch, ok.

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

ardumont created this revision.Nov 22 2019, 4:18 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/244/ for more details.

Harbormaster completed remote builds in B9340: Diff 8069.Nov 22 2019, 4:23 PM

ardumont added inline comments.Nov 22 2019, 4:23 PM

swh/loader/package/tests/data/https_replicate.npmjs.com/@aller_shared
153	The correct version to lookup in the self.info['time'] below. And the version found in the package.json is '0.1.1-alpha.14+ac08282d'... So without the fix, the join fails.
305	here!

ardumont added inline comments.Nov 22 2019, 5:32 PM

swh/loader/package/tests/data/https_replicate.npmjs.com/@aller_shared
153	(i mean the package.json within the source archive)

ardumont edited the summary of this revision. (Show Details)Nov 22 2019, 5:33 PM

ardumont retitled this revision from npm: Fix npm loading failure on divergent versions referenced in package.json to npm: Fix npm loading failure on different versions referenced in package.json.Nov 22 2019, 5:41 PM

Looks good!

swh/loader/package/tests/test_npm.py
539	Here, I would rather use the [[ https://docs.pytest.org/en/latest/fixture.html#using-fixtures-from-classes-modules-or-projects \| `usefixtures` ]] decorator from `pytest` as there is no direct access to the `requests_mock_datadir` fixture in the test implementation. @pytest.mark.usefixtures("requests_mock_datadir") def test_npm_loader_version_divergence(swh_config):

This revision is now accepted and ready to land.Nov 22 2019, 5:42 PM

ardumont added inline comments.Nov 22 2019, 5:46 PM

swh/loader/package/tests/test_npm.py
539	Nice, thanks for the heads up on this.

ardumont added inline comments.Nov 22 2019, 5:53 PM

swh/loader/package/tests/test_npm.py
539	https://docs.pytest.org/en/latest/reference.html#pytest-mark-usefixtures

I think we should always avoid retrieving data from the API using the (potentially mismatched) intrinsic version number, but always using the one that the API claims.

Suppose the API presents two versions of the package:

0.1.0, intrinsic version 0.1.0, extrinsic date d0
0.2.0, (buggy) intrinsic version 0.1.0 as well, extrinsic date d1

Then retrieving the metadata for version 0.2.0 would come up with date d0, instead of the expected date d1.

It would also be useful to update the comment to be clearer (it took me a while to understand it). Something along the lines of :

# No date available in intrinsic metadata: retrieve it from the API metadata, using the version number that the API claims this package has.

agreed!

Update according to latest exchange

ardumont retitled this revision from npm: Fix npm loading failure on different versions referenced in package.json to npm: Explicitely retrieve the revision date from extrinsic metadata.Nov 22 2019, 6:03 PM

ardumont edited the summary of this revision. (Show Details)

Build is green
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/245/ for more details.

Harbormaster completed remote builds in B9343: Diff 8072.Nov 22 2019, 6:04 PM

ardumont edited the test plan for this revision. (Show Details)Nov 22 2019, 6:08 PM

ardumont added projects: Core Loader, Npm loader, Origin-npm.

olasd accepted this revision.Nov 22 2019, 6:10 PM

Closed by commit rDLDBASE105e76d79a00: npm: Explicitely retrieve the revision date from extrinsic metadata (authored by ardumont). · Explain WhyNov 22 2019, 6:11 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDLDBASE105e76d79a00: npm: Explicitely retrieve the revision date from extrinsic metadata.