-
Notifications
You must be signed in to change notification settings - Fork 26
feat: --package-installer option #708
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?
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 |
|---|---|---|
|
|
@@ -47,6 +47,9 @@ def __init__( | |
| # Fields that are not loaded from the environment subprocess | ||
| self.python_version_requirement = python_version_requirement | ||
| self.python_interpreter = python_interpreter | ||
| # Optional override of server install behavior. If None, server-driven | ||
| # default is used. | ||
| self.package_manager_allow_uv: typing.Optional[bool] = None | ||
|
|
||
| def __getattr__(self, name: str) -> typing.Any: | ||
| # We directly proxy the attributes of the EnvironmentData object | ||
|
|
@@ -56,7 +59,7 @@ def __getattr__(self, name: str) -> typing.Any: | |
| def __setattr__(self, name: str, value: typing.Any) -> None: | ||
| if name in self.DATA_FIELDS: | ||
| # proxy the attribute to the underlying EnvironmentData object | ||
| self._data._replace(**{name: value}) | ||
| self._data = self._data._replace(**{name: value}) | ||
| else: | ||
| super().__setattr__(name, value) | ||
|
|
||
|
|
@@ -101,6 +104,7 @@ def create_python_environment( | |
| python: typing.Optional[str] = None, | ||
| override_python_version: typing.Optional[str] = None, | ||
| app_file: typing.Optional[str] = None, | ||
| package_manager: typing.Optional[str] = None, | ||
|
Collaborator
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. Would a |
||
| ) -> "Environment": | ||
| """Given a project directory and a Python executable, return Environment information. | ||
|
|
||
|
|
@@ -153,6 +157,14 @@ def create_python_environment( | |
| # that didn't support environment.python.requires | ||
| environment.python = override_python_version | ||
|
|
||
| if package_manager is not None: | ||
| if package_manager not in ("pip", "uv"): | ||
| raise RSConnectException("Unsupported package manager: %s" % package_manager) | ||
|
Comment on lines
+161
to
+162
Collaborator
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. Changing
Collaborator
Author
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. Fair, I retained the pattern the code has of checking values at runtime, |
||
| # Override the package manager name recorded by inspector | ||
| environment.package_manager = package_manager # type: ignore[attr-defined] | ||
| # Derive allow_uv from selection | ||
| environment.package_manager_allow_uv = True if package_manager == "uv" else False | ||
|
Collaborator
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. Trying to confirm my understanding... Is
Collaborator
Author
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.
|
||
|
|
||
| if force_generate: | ||
| _warn_on_ignored_requirements(directory, environment.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.
😱 This was a bug?
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.
yep... somehow we never faced it, but the data was indeed never updated