-
Notifications
You must be signed in to change notification settings - Fork 2.4k
cache: automatic refresh if files of locked package differ from files in cache #10657
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
base: main
Are you sure you want to change the base?
Conversation
… in cache When working on a project with many developers unnecessary diffs in the lockfile due to outdated caches of some developers are an annoying issue. Outdated caches are normally the result of postponed artifact uploads. This change detects such diffs and refreshes the cache automatically.
Reviewer's GuideAdds automatic cache refresh for locked packages when their file lists differ from cached repository data, plus supporting repository APIs and tests to ensure outdated caches are corrected and not refreshed unnecessarily. Sequence diagram for automatic cache refresh on package file mismatchsequenceDiagram
participant Provider
participant RepositoryPool
participant CachedRepository
participant ReleaseCache as Release_cache
Provider->>RepositoryPool: package(pretty_name, version, repository_name)
RepositoryPool->>CachedRepository: package(name, version)
CachedRepository->>ReleaseCache: get_release_info(canonical_name, version)
ReleaseCache-->>CachedRepository: release_info
CachedRepository-->>RepositoryPool: Package(pool_package)
RepositoryPool-->>Provider: Package(pool_package)
Provider->>Provider: compare package.files with pool_package.files
alt file lists differ
Provider->>RepositoryPool: refresh(pool_package)
RepositoryPool->>RepositoryPool: determine repository_name
RepositoryPool->>CachedRepository: forget(name, version)
CachedRepository->>ReleaseCache: forget(canonical_name:version)
ReleaseCache-->>CachedRepository: removed
RepositoryPool->>CachedRepository: package(name, version)
CachedRepository->>ReleaseCache: get_release_info(canonical_name, version)
ReleaseCache-->>CachedRepository: fresh_release_info
CachedRepository-->>RepositoryPool: Package(refreshed_package)
RepositoryPool-->>Provider: Package(refreshed_package)
Provider->>Provider: add key to _refreshed
else file lists equal
Provider->>Provider: keep pool_package
end
Provider->>Provider: create DependencyPackage and continue resolution
Class diagram for cache refresh of locked packagesclassDiagram
class Provider {
+RepositoryPool _pool
+Callable get_package_from_pool
+set~tuple~ _refreshed
+complete_package(package, dependency) DependencyPackage
}
class RepositoryPool {
+list~Repository~ repositories
+package(name, version, repository_name) Package
+repository(name) Repository
+search(query) list~Package~
+refresh(package) Package
}
class CachedRepository {
+dict _release_cache
+package(name, version) Package
+forget(name, version) void
}
class Package {
+str pretty_name
+Version version
+list~dict~ files
+str source_reference
+str name
}
class DependencyPackage {
+Dependency dependency
+Package package
}
class Dependency {
+str source_name
}
class Repository {
<<interface>>
+package(name, version) Package
+search(query) list~Package~
}
Provider --> RepositoryPool : uses
RepositoryPool --> Repository : aggregates
Repository <|-- CachedRepository
Provider --> DependencyPackage : creates
DependencyPackage --> Package
DependencyPackage --> Dependency
RepositoryPool --> Package : refresh(package)
CachedRepository --> Package : package(name, version)
CachedRepository --> Package : forget(name, version)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The
_refreshedset inProvidergrows monotonically for the lifetime of the provider; consider a strategy to bound or periodically clear it (or key it more narrowly) to avoid unbounded memory growth on large or long-running resolves. - In
RepositoryPool.refresh, the fallback to the literal "PyPI" whensource_referenceis missing relies on that exact repository name being present; it may be safer to reuse the same defaulting logic as other pool lookups or make the default repository explicit at construction time to avoid subtle mismatches. - There are two very similar
MockCachedRepositorytest doubles (intests/puzzle/test_provider.pyandtests/repositories/test_cached_repository.py); consider extracting a shared helper or fixture to reduce duplication and keep behavior aligned if one evolves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_refreshed` set in `Provider` grows monotonically for the lifetime of the provider; consider a strategy to bound or periodically clear it (or key it more narrowly) to avoid unbounded memory growth on large or long-running resolves.
- In `RepositoryPool.refresh`, the fallback to the literal "PyPI" when `source_reference` is missing relies on that exact repository name being present; it may be safer to reuse the same defaulting logic as other pool lookups or make the default repository explicit at construction time to avoid subtle mismatches.
- There are two very similar `MockCachedRepository` test doubles (in `tests/puzzle/test_provider.py` and `tests/repositories/test_cached_repository.py`); consider extracting a shared helper or fixture to reduce duplication and keep behavior aligned if one evolves.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When working on a project with many developers, unnecessary diffs in the lockfile due to outdated caches of some developers are an annoying issue. Outdated caches are normally the result of postponed artifact uploads. This change detects such diffs and refreshes the cache automatically.
Pull Request Check List
Summary by Sourcery
Automatically refresh cached package metadata when lockfile files differ from repository cache to avoid unnecessary lockfile diffs.
New Features:
Bug Fixes:
Enhancements:
Tests: