-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: allow a folder with a list of wheels to be a repository #10017
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
from __future__ import annotations | ||
|
||
import logging | ||
from typing import Iterator | ||
from pathlib import Path | ||
|
||
from cachecontrol.controller import logger as cache_control_logger | ||
from poetry.core.packages.package import Package | ||
from poetry.core.packages.utils.link import Link | ||
from poetry.repositories.repository import Repository | ||
from poetry.inspection.info import PackageInfo | ||
from poetry.core.packages.utils.utils import path_to_url | ||
|
||
cache_control_logger.setLevel(logging.ERROR) | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class LocalPathRepository(Repository): | ||
def __init__(self, name: str, path: str) -> None: | ||
self._path = Path(path) | ||
if not self._path.is_dir(): | ||
raise ValueError(f"Path {self._path} is not a directory") | ||
|
||
super().__init__(name, self._list_packages_in_path()) | ||
|
||
@property | ||
def path(self) -> str: | ||
return self._path | ||
|
||
def _list_packages_in_path(self) -> Iterator[Package]: | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
)
continue You'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. |
||
continue | ||
|
||
def find_links_for_package(self, package: Package) -> list[Link]: | ||
return [Link(path_to_url(package.source_type))] | ||
Comment on lines
+38
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
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.
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.