Support environment.python.requires option in manifest#648
Conversation
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
| manifest["files"] = {} | ||
|
|
||
| return manifest | ||
| manifest: Manifest = Manifest( |
There was a problem hiding this comment.
All places where we were manually creating ManifestData have been migrated to use the Manifest class. This ensures that the same behaviour is applied independently from the command used, and thus that the python version requirements are always applied to the manifest without having to set them in every single place.
This is also more robust to future changes to the manifest.
| return exists(python_path) or exists(win_path) | ||
|
|
||
|
|
||
| def list_environment_dirs(directory: str | Path): |
There was a problem hiding this comment.
Everything related to environments has been moved into rsconnect.environment
| def _replace(self, **kwargs: object): | ||
| return replace(self, **kwargs) | ||
|
|
||
| class Environment: |
There was a problem hiding this comment.
I tried to improve separation of concerns isolating the detection of the environment state in subprocesses.inspect_environment which will only focus on inspecting the environment and return EnvironmentData in JSON.
Then the developer is expected to always only interact with Environment class which invokes inspect_environment itself. The EnvironmentData is considered an internal implementation detail and only acts as the communication protocol between Environment and inspect_environment.
Every time a python environment has to be describe, the developer is expected to build an Environment object, usually by invoking Environment.create_python_environment which is the preferred factory method that takes care of inspecting an existing envrionment and creating an Environment object out of it.
| @@ -0,0 +1,235 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
Didn't change, just renamed from rsconnect.environment
| import toml as tomllib # type: ignore[no-redef] | ||
|
|
||
|
|
||
| def detect_python_version_requirement(directory: typing.Union[str, pathlib.Path]) -> typing.Optional[str]: |
There was a problem hiding this comment.
This is more or less the entry point for the whole version requirement detectiong process, based on the previously merged metadata parsing functions.
|
There are the two failures that are also happening on |
Lytol
left a comment
Lytol
left a comment
There was a problem hiding this comment.
Outstanding! Really appreciate the refactor of both the Environment class as well as extracting the environment inspection into subprocesses. I did not review line-by-line on the refactors that were simply moving things around, so I'm trusting that the test coverage is enough there.
My comments are trivial nits and/or copy edits, so do with them what you'd like.
README.md
Outdated
| if no `.python-version` file exists. | ||
| 3. A `setup.cfg` with a `options.python_requirs` field exists. | ||
| In such case the requirement specified in the field will be used | ||
| if no `.python-version` or `pyproject.toml` files exist. |
There was a problem hiding this comment.
Nit: The requirement might still be used if pyproject.toml exists, but only if it doesn't specify a project.requires-python. This is pretty nuanced though, and I think it's fine to leave it as is.
README.md
Outdated
| 4. If no other source of version requirement was found, then | ||
| the interpreter in use is considered the one required to run the content. | ||
|
|
||
| On newer Posit Connect versions the requirement detected by `rsconnect` is |
There was a problem hiding this comment.
Should we specify what "newer Posit Connect versions" means here? Specifically, I think it'll be versions >=2025.03.0.
| We recommend providing a `pyproject.toml` with a `project.requires-python` field | ||
| if the deployed content is an installable package and a `.python-version` file | ||
| for plain directories. |
There was a problem hiding this comment.
❤️ It's really nice to provide specific guidance like this.
Co-authored-by: Brian Smith <brian.e.smith@gmail.com>
2 participants
Intent
Connect supports python interpreter version requirements from the
environment.python.requiresfield in the manifest.That field should be filled based on the version requirements in the project metadata files:
Type of Change
Approach
Environmentinformation via the same code pathAutomated Tests
environment.python.requiresfield when detectedDirections for Reviewers
--override-python-versionstill allows overwriting the detected version, but on long term the plan is to deprecate the option and ask users to create a.python-versionfile to do the same. So that is a more standard approach in the Python ecosystem.Checklist