feat: allow a folder with a list of wheels to be a repository #10017
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new feature that allows using a local directory containing wheel files as a repository. This is particularly useful for offline work, containerized environments, or CI/CD pipelines where direct internet access might be restricted. Sequence diagram for adding a local wheelhouse repositorysequenceDiagram
actor User
participant CLI as Poetry CLI
participant Factory as Poetry Factory
participant Config as Poetry Config
participant Repo as LocalPathRepository
User->>CLI: poetry source add wheelhouse --path=./wheelhouse
CLI->>Factory: create_package_source()
Factory->>Config: validate source config
Factory->>Repo: create LocalPathRepository
Repo->>Repo: validate directory
Repo->>Repo: scan for wheel files
Repo-->>Factory: return repository instance
Factory-->>CLI: repository added
CLI-->>User: success message
Class diagram for the new LocalPathRepository implementationclassDiagram
class Repository {
<<abstract>>
+name: str
+packages: list
}
class LocalPathRepository {
-_path: Path
+path: str
+__init__(name: str, path: str)
-_list_packages_in_path(): Iterator[Package]
+find_links_for_package(package: Package): list[Link]
}
class Source {
+name: str
+url: str
+path: str
+priority: Priority
}
Repository <|-- LocalPathRepository
note for LocalPathRepository "New repository type for local wheel directories"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @florianvazelle - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add documentation for the new local path repository feature, including configuration options and usage examples.
- Tests should be added to verify the local path repository functionality, including error cases for malformed wheels and invalid paths.
- Consider improving error handling in LocalPathRepository._list_packages_in_path() to provide more informative errors when processing invalid wheel files.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def find_links_for_package(self, package: Package) -> list[Link]: | ||
| return [Link(path_to_url(package.source_type))] |
There was a problem hiding this comment.
issue (bug_risk): The find_links_for_package method appears to be using the wrong attribute for path generation
Using package.source_type (which is typically a string like 'directory' or 'file') as a path will likely cause errors. Consider using package.source_url or constructing the correct path from self._path and the package filename.
| try: | ||
| url = source["url"] | ||
| except KeyError: | ||
| raise InvalidSourceError(f"Missing [url] in source {name!r}.") | ||
|
|
||
| repository_class = LegacyRepository | ||
| try: | ||
| path = source["path"] | ||
| except KeyError: | ||
| raise InvalidSourceError(f"Missing [url] or [path] in source {name!r}.") | ||
| else: | ||
| return LocalPathRepository(name, path) |
There was a problem hiding this comment.
suggestion: Consider validating that only one of 'url' or 'path' is provided in source configuration
When both 'url' and 'path' are provided, the code silently ignores 'path'. Consider adding validation to explicitly handle this case and provide clear feedback to users.
| for file_path in self._path.iterdir(): | ||
| try: | ||
| yield PackageInfo.from_path(path=file_path).to_package(root_dir=file_path) | ||
| except Exception: |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid catching bare Exception without logging
Catching all exceptions silently could hide important errors. Consider logging the exceptions to help with debugging and potentially catch more specific exceptions.
Suggested implementation:
try:
yield PackageInfo.from_path(path=file_path).to_package(root_dir=file_path)
except (ValueError, OSError) as e:
logger.warning(
"Failed to load package info from %s: %s", file_path, str(e)
)
continueYou'll also need to add a logger instance at the top of the file if it doesn't exist already:
logger = logging.getLogger(__name__)The exceptions I've chosen (ValueError, OSError) are common ones that might occur when reading package info, but you may want to adjust these based on the specific exceptions that PackageInfo.from_path() and to_package() can raise.
Pull Request Check List
Resolves: #5983
Feature Request
Hi 👋
I submit a PR to allow a folder with a list of wheels (or sdist) to be a repository.
This is usefull to work offline with wheelhouse, for container or CI stuff.
I work on a POC that allow to set sources, like this:
I'm open to feedback 🙂
Minimal, Reproducible Example
With a
pyproject.toml, like:Run these commands to setup a wheelhouse (using poetry):
And now regenerate the lockfile with the wheelhouse as source:
This produces a lockfile, like this:
And the dependency can normally be installed.
Summary by Sourcery
New Features: