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

Fix repository cache inconsistency issue #6157

Closed

Conversation

radoering
Copy link
Member

Fixes an inconsistency when caching matches of a LegacyRepository. Without the fix find_packages() might return an empty list on second call for packages with only pre-releases because only (released) versions are cached but not ignored_pre_release_versions (which cannot be ignored at all events).

  • Added tests for changed code.
  • Updated documentation for changed code.

@radoering radoering requested a review from a team August 12, 2022 17:40
@dimbleby
Copy link
Contributor

I fixed the same thing incidentally in #6081; the changes to the legacy repository there could be taken standalone.

Our fixes are quite different: I was mostly trying to make the legacy and pypi repositories more alike and it was almost accidental that I discovered this bug. The fix here is certainly more targeted, but it does indeed make the repository classes diverge yet further.

I'm pretty suspicious that this cache is more or less worthless anyway - #5868 (comment) - probably it could be removed without anyone noticing the difference...

@radoering
Copy link
Member Author

I like making find_packages of the different repositories more alike. I'd like to extract the changes to LegacyRepository from #6081. Do you want to create a PR or are you fine if I adopt them?

There is just one thing I can't assess yet: You removed some replace('.', '-'). Can you explain the reason?

I'm pretty suspicious that this cache is more or less worthless anyway - #5868 (comment) - probably it could be removed without anyone noticing the difference...

Same feeling, but not sure enough...

@dimbleby
Copy link
Contributor

I'm more than fine with you adopting those changes, help yourself.

I removed the replace('.', '-') stuff because it is made redundant by #6022: canonicalized names have already made this replacement.

@radoering
Copy link
Member Author

Preferred alternative PR: #6165

@dimbleby: Please take a look.

@radoering
Copy link
Member Author

Superseded by #6165

@radoering radoering closed this Aug 15, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
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.

2 participants