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
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9343
Build 13680: tox-on-jenkinsJenkins
Build 13679: arc lint + arc unit

Event Timeline

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!

swh/loader/package/tests/data/https_replicate.npmjs.com/@aller_shared
153

(i mean the package.json within the source archive)

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 added a subscriber: anlambert.

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
swh/loader/package/tests/test_npm.py
539

Nice, thanks for the heads up on this.

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.

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)