-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Implement explicit environment spec file support based on CEP 23
.
#14820
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
Conversation
CodSpeed Performance ReportMerging #14820 will not alter performanceComparing Summary
|
Creating an environment using this plugin requires a solve. A solve is not required when using Additionally CEP-23 states:
|
Testing this out it looks like all explicit files are detected as being handled by the
|
Yeah, something is clearly wrong, will fix! |
CEP 23
.
conda/env/installers/conda.py
Outdated
# Handle explicit environments separately per CEP-23 requirements | ||
if isinstance(env, ExplicitEnvironment): | ||
return _install_explicit_environment(prefix, specs, env) |
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 is a special case but one that will come up for any plugins that reads a definition for a locked environment (including pixi.lock or conda-lock.yml files). In these cases a solve is unnecessary, should not be performed and in some cases (e.g. packages outside of a channel are referenced) may not be possible. I think providing a path for plugins to express this information would be useful but likely requires an expansion of the Environment
class.
For pixi.lock and conda-lock.yml, parsing can return a set of PackageRecord
instances for the conda requirements, where as requirement and environment.yaml files would return a set of MatchSpec
instances.
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.
Should we have this information being a metadata in the CondaEnvironmentSpecifier
class?
@dataclass
class CondaEnvironmentSpecifier:
"""
Return type to use when defining a conda env spec plugin hook.
For details on how this is used, see
:meth:`~conda.plugins.hookspec.CondaSpecs.conda_environment_specifiers`.
:param name: name of the spec (e.g., ``environment_yaml``)
:param environment_spec: EnvironmentSpecBase subclass handler
:param requires_solve: Whether or not the plugin requires a solve
"""
name: str
environment_spec: type[EnvironmentSpecBase]
requires_solve: bool
and then
@hookimpl
def conda_environment_specifiers():
yield CondaEnvironmentSpecifier(
name="explicit",
environment_spec=ExplicitRequirementsSpec,
requires_solve=False,
)
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.
Another option would be to attaching this metadata to the Environment
class itself which is returned from EnvironmentSpecBase.environment
method.
Either option is reasonable and will allow for plugins for various lock formats.
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.
I agree, I think given that we may have an evolved environment class in the future, it might make sense to have it there, instead of the plugin interface. Thanks for the idea!
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.
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.
Is there a need to get this feature ASAP? If not I'd prefer to wait until the Environment class is merged. Once that work is done this can be re-worked to use that class.
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.
Yeah, I think we've been stalling on the features that were already well-defined, while the environment class refactor has been ballooning already. This branch here works now, and is a good snapshot in time.
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.
@jaimergp that's the similar conversation you had above, I'll change to the boolean in the environment class
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.
Neat! This works when I (pull in the changes from #14725 and) try to reproduce the issue described in #14725 (comment).
I wonder if we should merge those PRs together? |
For me, its ok if they are seperate since they implementation doesn't overlap and they have 2 distinct goals. But, if you think it would be helpful to merge them, that sounds fine as well. |
In general, I think this looks good to me and seems to work in all the ways I expect it to. I want to call out 2 other things happening in parallel here:
|
Co-authored-by: Mahe Iram Khan <65779580+ForgottenProgramme@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
- Introduced `is_url` and `expand` functions for improved URL normalization and path expansion. - Updated logic to handle file paths and URLs consistently, ensuring compatibility with existing specifications. - Added validation for URL format using regex, maintaining backward compatibility while improving parsing accuracy.
- Removed the ExplicitEnvironment class and integrated its functionality into the Environment class. - Updated the Dependencies class to include an explicit marker for environments created from @explicit files. - Adjusted the conda installer and requirements specifications to work with the new Environment structure. - Modified tests to reflect changes in the environment handling, ensuring compatibility with explicit specifications.
…cations - Replaced the ExplicitRequirementsSpec with a unified RequirementsSpec that handles both regular and explicit requirements files. - Updated the reading logic to utilize a generator for improved efficiency and clarity. - Adjusted tests to reflect the new unified handling of requirements, ensuring compatibility with explicit specifications. - Removed deprecated functions and streamlined the codebase for better maintainability.
- Integrated explicit environment handling directly into the install function, removing the need for a separate _install_explicit_environment function. - Enhanced logging to provide detailed feedback on the source of package specifications used during installation. - Improved the logic for determining package specs by prioritizing original files, provided specs, and environment dependencies. - Ensured compatibility with the latest requirements handling and streamlined the installation process for explicit environments.
- Removed the _execute_transaction function and integrated its logic directly into the install function. - Enhanced the transaction handling by checking for 'nothing to do' before executing. - Streamlined the process of downloading, extracting, and executing transactions, improving clarity and maintainability.
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
…ror if no solver backend is found.
…ng for better performance and readability. - Removed unnecessary comments and improved the `yield_lines` function in `read.py` for cleaner code.
- Updated the `explicit` function to track user-requested specs separately from all packages, preventing dependencies from polluting the user's request history. - Added type hints for parameters in `explicit`, `_solve`, `dry_run`, and `install` functions to improve code clarity and type checking. - Modified test cases to ensure correct handling of the `@EXPLICIT` marker in various positions within the dependency list.
- Removed redundant fixture and consolidated tests for explicit environments. - Added new tests to verify the detection of explicit dependencies and the handling of the @explicit marker in various positions within the dependency list. - Improved type hinting for test functions and ensured consistent assertions for environment properties.
- Updated assertions in `test_explicit_file_spec_is_registered` to improve readability and ensure correct handling of dependencies. - Replaced direct access to `env.dependencies["conda"]` with a more general approach using `env.dependencies.raw`, enhancing flexibility in dependency checks.
33b11f7
to
58fb7dc
Compare
…ested specs are recorded, preventing dependencies from affecting the user's request history. Also enhanced the handling of requested specs by using `MatchSpec` for better specification matching.
Description
explicit
input file types conda-planning#47Checklist - did you ...
news
directory (using the template) for the next release's release notes?