Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes registry used to check for deprecation settings #5534

Merged
merged 5 commits into from
Jun 29, 2023

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Jun 26, 2023

What's the problem this PR addresses?

The new yarn npm audit implementation checks the packages' metadata on the audit server rather than the server configured to serve their metadata.

Closes #5533

How did you fix it?

Updated the code to use the new getPackageMetadata function, which should use the proper server while also leveraging the cache if possible.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a test and getPackageMetadata doesn't include the deprecated field.
#5491 (comment)

Copy link
Member

@paul-soporan paul-soporan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecations shouldn't use getPackageMetadata - even if we cached the deprecated field, it could be stale for exact versions since it can change at any time.

@arcanis
Copy link
Member Author

arcanis commented Jun 26, 2023

even if we cached the deprecated field, it could be stale for exact versions since it can change at any time

In which case I'd expect the registry to fail the etag / last modified check and return the updated metadata. I don't pass a version number, so we should never return straight from the cache.

I didn't notice that the deprecated field wasn't cached though - I'm thinking of adding it, as having a working cache on audit should make it much faster - the deprecation check is currently the slower part.

@arcanis arcanis merged commit b39e673 into master Jun 29, 2023
16 of 17 checks passed
@arcanis arcanis deleted the mael/audit-deprecated-registry branch June 29, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn npm audit cannot find packages from custom registry server
3 participants