fix(pypi): lookup simple api first #26505
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
secustor
reviewed
Jan 4, 2024
secustor
requested changes
Jan 6, 2024
lib/modules/datasource/pypi/index.ts
Outdated
| } | ||
| return dependency; | ||
| // merge results | ||
| return Object.assign({}, simpleDependencies, pypiJsonDependencies); |
There was a problem hiding this comment.
Suggested change
| return Object.assign({}, simpleDependencies, pypiJsonDependencies); | |
| return { | |
| ...simpleDependencies, | |
| ...pypiJsonDependencies | |
| }; |
Will all pypiJsonDependencies the same information as the simple ones.
As this will overwrite releases.
There was a problem hiding this comment.
You are right. Maybe concatenating the found releases is better? If I saw it correctly, the dependencies should be de-duplicated later
There was a problem hiding this comment.
I prefer to deduplicate in the datasource
Co-authored-by: Sebastian Poxhofer <[email protected]>
viceice
reviewed
Jan 7, 2024
viceice
reviewed
Jan 30, 2024
This comment has been minimized.
This comment has been minimized.
rarkins
reviewed
May 7, 2024
| simpleHostUrl, | ||
| ).catch((err) => { | ||
| if (err.statusCode !== 404) { | ||
| throw err; |
There was a problem hiding this comment.
This may be too aggressive. Custom registries can sometimes return weird things and we don't necessarily want to give up
rarkins
requested changes
May 7, 2024
Comment on lines
+79
to
+84
| // merge results | ||
| return { | ||
| releases: [], | ||
| ...simpleDependencies, | ||
| ...pypiJsonDependencies, | ||
| }; |
|
@Kakadus do you have any time to resume this PR? |
|
Closing due to inactivity and conflicts. Reopening or replacing this PR in future would be welcomed |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This rewrites the
getReleasesfunction of pypi datasources:First, the the simple (pypi) api is queried, then the pypijson api is retried on 404s or to enrich the metadata.
For that, I adapted the formerly named
getDependenciesfunctions to add their resolved data to an existing Result.Previously, the simple api was only used, if pypijson returned 404.
The unit tests stayed mostly the same, but the simple api routes needed to be mocked. Also added testcases to check that both simple and json pypi apis are used, and dependency resolution succeeds if json api is unavailable.
Context
Closes #26483.
As pypijson is used additionally, #26383 is resolved as well.
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: