Replace DelegatedConda with Delegated #5963
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
trexfeathers
added
the
benchmark_this
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5963 +/- ##
=======================================
Coverage 89.78% 89.78%
=======================================
Files 93 93
Lines 23010 23010
Branches 5018 5018
=======================================
Hits 20660 20660
Misses 1620 1620
Partials 730 730 ☔ View full report in Codecov by Sentry. |
⏱️ Performance Benchmark Report: f5bf5b7Performance shiftsFull benchmark resultsGenerated by GHA run |
trexfeathers
added
benchmark_this
⏱️ Performance Benchmark Report: 7c75a23Performance shiftsFull benchmark resultsGenerated by GHA run |
trexfeathers
commented
May 21, 2024
Comment on lines
+40
to
+41
| (first_command,) = environment._interpolate_commands(raw_commands[0]) | ||
| command, env, return_codes, cwd = first_command |
There was a problem hiding this comment.
Following the pattern in ASV's own source.
Comment on lines
+131
to
+156
| def __init__(self, conf, python, requirements, tagged_env_vars): | ||
| """Get a 'delegated' environment based on the given ASV config object. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| conf : dict | ||
| ASV configuration object. | ||
|
|
||
| python : str | ||
| Ignored - environment management is delegated. The value is always | ||
| ``DELEGATED``. | ||
|
|
||
| requirements : dict (str -> str) | ||
| Ignored - environment management is delegated. The value is always | ||
| an empty dict. | ||
|
|
||
| tagged_env_vars : dict (tag, key) -> value | ||
| Ignored - environment management is delegated. The value is always | ||
| an empty dict. | ||
|
|
||
| Raises | ||
| ------ | ||
| EnvironmentUnavailable | ||
| The original environment or delegated environment cannot be created. | ||
|
|
||
| """ |
There was a problem hiding this comment.
| self._path_delegated.symlink_to(delegated_env_path, target_is_directory=True) | ||
| assert self._delegated_found | ||
|
|
||
| def _setup(self): |
There was a problem hiding this comment.
| else: | ||
| env_parent = env_prep.env_parent | ||
|
|
||
| # See :meth:`Environment._interpolate_commands`. |
There was a problem hiding this comment.
benchmarks/asv_delegated.py
Outdated
Comment on lines
277
to
281
| # Run the script(s) for delegated environment creation/updating. | ||
| # (An adaptation of :meth:`Environment._interpolate_commands`). | ||
| for command, env, return_codes, cwd in self._interpolate_commands( | ||
| env_prep.commands | ||
| ): |
There was a problem hiding this comment.
Following the pattern in ASV's own source.
Comment on lines
+334
to
+337
| if not self._delegated_found: | ||
| # Required during environment setup. OSError expected if executable | ||
| # not found. | ||
| raise OSError |
There was a problem hiding this comment.
Comment on lines
+347
to
+350
| def run(self, args, **kwargs): | ||
| # This is not a specialisation - just implementing the abstract method. | ||
| log.debug(f"Running '{' '.join(args)}' in {self.name}") | ||
| return self.run_executable("python", args, **kwargs) |
There was a problem hiding this comment.
trexfeathers
commented
May 22, 2024
There was a problem hiding this comment.
For exploring this space I recommend something like the below:
from asv.config import Config
from asv.repo import get_repo, Repo
from asv_delegated import Delegated
iris_config = Config.load("benchmarks/asv.conf.json")
iris_repo: Repo = get_repo(iris_config)
delegated_instance = Delegated(iris_config, "3.12", {}, {})
# Can now try out different methods to see what happens.
# Run in isolation like this, they might not work as intended, but you can
# debug to demonstrate operations / see variable values / etc.
delegated_instance.checkout_project(iris_repo, "1d064e39")
delegated_instance.run_executable("python", ["-c", "print('Hello, World!')"])
trexfeathers
commented
Jun 3, 2024
| message = f"No env prep script available for commit: {commit_hash} ." | ||
| raise KeyError(message) | ||
| else: | ||
| parentage = dict(sorted(parentage.items(), key=lambda item: item[1])) |
There was a problem hiding this comment.
This is the recommended way of sorting a Python dictionary based on its values:
sorted(parentage.items(), key=lambda item: item[1])
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚀 Pull Request
Description
This new approach to managing our benchmark environments provides:
asv.plugins.conda.Conda, which merited several adaptations to get that particular setup to work. Now we just subclass the coreasv.environments.Environment.asv.conf.jsonstructure to avoid 'coupling' between separate config items.asv.conf.jsonmakes it possible to benchmark commits that need different environment setup commands (e.g. different Nox calls for different Python versions).Consult Iris pull request check list
Add any of the below labels to trigger actions on this PR: