Page MenuHomeSoftware Heritage

npm: Explicitely retrieve the revision date from extrinsic metadata
ClosedPublic

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

Details

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
  • tox
  • 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
ardumont added inline comments.Nov 22 2019, 4:23 PM
swh/loader/package/tests/data/https_replicate.npmjs.com/@aller_shared
154

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.

306

here!

ardumont added inline comments.Nov 22 2019, 5:32 PM
swh/loader/package/tests/data/https_replicate.npmjs.com/@aller_shared
154

(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
anlambert accepted this revision.Nov 22 2019, 5:42 PM
anlambert added a subscriber: anlambert.

Looks good!

swh/loader/package/tests/test_npm.py
540

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
540

Nice, thanks for the heads up on this.

olasd added a subscriber: olasd.Nov 22 2019, 5:56 PM

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.
ardumont updated this revision to Diff 8072.Nov 22 2019, 6:02 PM

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)
ardumont edited the test plan for this revision. (Show Details)Nov 22 2019, 6:08 PM
olasd accepted this revision.Nov 22 2019, 6:10 PM