Skip to content

workaround pypi empty responses by disabling cache control#4823

Closed
kapilt wants to merge 5 commits into
python-poetry:masterfrom
kapilt:kapilt/workaround-cachecontrol-pypi-issues
Closed

workaround pypi empty responses by disabling cache control#4823
kapilt wants to merge 5 commits into
python-poetry:masterfrom
kapilt:kapilt/workaround-cachecontrol-pypi-issues

Conversation

@kapilt
Copy link
Copy Markdown

@kapilt kapilt commented Nov 24, 2021

Pull Request Check List

Resolves: #4821
Alternative to: #4822 and #4717

This is a work-around fix for pypi/warehouse#10387 . Thanks @j-martin for also looking at this.

This disables cache control for package metadata due to issues observed that pypi can return empty responses. The package artifact is still maintained. The diff is a little larger due to need to keeping type checker happy and removing tests assuming cache control usage.

  • [ x ] Added tests for changed code.
  • [ x ] Updated documentation for changed code.

Comment thread src/poetry/repositories/legacy_repository.py Outdated
)

self._cache_control_cache = FileCache(str(release_cache_dir / "_http"))
self._session = CacheControl(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not a 100% convinced this is the right solution for this. I'd much rather have e-tag used in the cache key.

Skipping cache for empty responses or invalid cache values, should be handled by gracefully handling failure - ie. clear the invalid cache and retry or fallback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess that's the naive approach here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That approach is at the request level and not really at the cache level. I think an intermediate approach (e.g. layered on top of CacheControl and factoring in ETAGS) is the way to go. The caching is an important optimization, and adding a retry system at the high-level HTTP (e.g. .get()) API seems more a hack than a fix.

Comment thread tests/fixtures/git/github.com/demo/demo/demo.egg-info/PKG-INFO Outdated
@kapilt
Copy link
Copy Markdown
Author

kapilt commented Feb 22, 2022

fwiw, i haven't actually hit this in a while using latest poetry releases.. closing, fwiw i think the actual issue was solved on the pypi cdn side.

@kapilt kapilt closed this Feb 22, 2022
@github-actions
Copy link
Copy Markdown

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.

Random locking failure due to CacheControl returning an empty bytes response

4 participants