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

feat(packagist): Support for v2 protocol #20626

Merged
merged 14 commits into from
Feb 28, 2023

Conversation

zharinov
Copy link
Collaborator

Changes

Context

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@zharinov
Copy link
Collaborator Author

Blocked by #20595

@zharinov zharinov marked this pull request as ready for review February 26, 2023 05:22
@zharinov
Copy link
Collaborator Author

The problem is that I can't test it on the real repository, however you can see the call to packagist.org is identical to what we do for v2 in general, once we have metadata-url.

@rarkins
Copy link
Collaborator

rarkins commented Feb 26, 2023

I had expected that we could remove the hardcoded logic for packagist.org because the new behavior would be to try v2 API first for all registries anyway. Can that be done, or have I misunderstood, or do you prefer to do it gradual?

@zharinov
Copy link
Collaborator Author

Well, I thought about query efficiency, but now I see it's not the case since we cache getRepositoryMeta() anyway

@rarkins
Copy link
Collaborator

rarkins commented Feb 26, 2023

  1. Try v2 first
  2. If it fails, and we can fall back, flag that both in memory as well as a longer term cache (e.g. 24 hours) so that at most we test v2 once per day per registryUrl

BTW it could also be written so that we "test" v2 first before doing an extra query, so that if there's 10 lookups at once then we don't get 10 failures/unnecessary queries

@zharinov
Copy link
Collaborator Author

zharinov commented Feb 26, 2023

I propose more straightforward yet potentially robust solution: always fetch metadata (/packages.json), cache it for 10 minutes and perform v2 queries when available, falling back to old methods otherwise.

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

lib/modules/datasource/packagist/index.ts Show resolved Hide resolved
@rarkins rarkins added this pull request to the merge queue Feb 28, 2023
Merged via the queue into renovatebot:main with commit c9fe3b9 Feb 28, 2023
@rarkins rarkins deleted the feat/packagist-v2-support branch February 28, 2023 09:53
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 34.154.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default composer/packagist to use v2 datasource API
4 participants