Repository navigation
feat(deploy_env): carry the universe artifact to the env provider - #122
Open
mandykwok-scale wants to merge 3 commits into
Open
mandykwok-scale wants to merge 3 commits into
mandykwok-scale wants to merge 3 commits into
Conversation
A warm environment pool keys on environment *and* universe — two deployments are only interchangeable when both match. Today `deploy_env` names only the env, and the universe arrives later via `load_artifact`, so a provider cannot tell which universe the run will use without inferring it from the taxonomy. `artifact_id` / `artifact_version` are optional and forwarded under the same signature guard as `litellm_api_key`: built-in envs take a fixed param list and would TypeError on an unexpected kwarg, so an env that does not declare them never sees them. A step that names no artifact passes none. Both are inert for every existing taxonomy. `load_artifact` is unchanged and still does the loading; this only tells the provider what is coming. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…name Two review findings. Slotting the new params next to `env_version` changed what an existing positional argument meant: a caller passing ttl_seconds fifth would have been setting artifact_id, and the env would have got the default TTL. They are appended now, so every existing position still means what it did. The guard checked for `artifact_id` and then sent both fields, so an env declaring the id but not the version — with no `**kwargs` to absorb it — would have raised on an unexpected keyword. Each field is gated on its own name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| super().__init__(id, version, depends_on=depends_on, fail_task_on_error=fail_task_on_error) | ||
| self.env_id = env_id | ||
| self.env_version = env_version | ||
| # The universe this deployment is for. Carried so a provider can see which artifact the |
Collaborator
There was a problem hiding this comment.
Can we remove this, since we should not assume that the artifact is a universe.
| self.env_id = env_id | ||
| self.env_version = env_version | ||
| # The universe this deployment is for. Carried so a provider can see which artifact the | ||
| # run will load, without inferring it from the taxonomy; load_artifact still loads it. |
Collaborator
There was a problem hiding this comment.
Lets remove this comment as well (leaking scale internals to public repo)
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
A warm environment pool keys on environment and universe — two deployments are only interchangeable when both match.
Today
deploy_envnames only the env. The universe arrives later, viaload_artifact, so anEnvironmentProviderhas no way to tell which universe a run will use. The alternative is inferring it by walking the taxonomy for theload_artifactstep, which guesses at something the DAG could simply state.Change
artifact_id/artifact_versionbecome optional params onDeployEnvTaskStep, forwarded toenv.deploy()and from there — viadeploy_through_provider(**options)— to the provider. The artifact is also declared as anEntityRef, so it is a tracked, version-pinnable reference likeenv_idalready is.Why this is inert for everything that exists
Forwarding reuses the signature guard already used for
litellm_api_key:Built-in envs take a fixed param list and would
TypeErroron an unexpected kwarg — which is what that guard exists for. An env that does not declareartifact_idnever receives it, and a step that names no artifact passes nothing.load_artifactis untouched and still does the loading. This only tells the provider what is coming.This does nothing on its own
No env declares
artifact_idyet, so the guard skips it every time. The companion PR adds it to the built-in envs and providers. Landing them separately keeps this reviewable as the pure contract addition it is.Test plan
Four unit tests in
tst/unit/task_step/deploy_env_test.py: forwarded to an env that accepts kwargs; omitted for an env with a fixed param list, which must not raise; absent when the step names no artifact; survives ato_dict/from_dictround trip.38 passed, 1 failed— the failure (test_preflight_reports_an_env_this_process_cannot_load[unregistered-type]) reproduces on a clean tree in this venv, whereagentenvhub_sdkregisters extra env types. Unrelated.🤖 Generated with Claude Code
The PR appears safe to merge; the latest edit does not change behavior.Summary
Deploy steps can now carry the artifact ID and version for the environment’s run, so an opted-in environment can receive that reference before
load_artifactloads it. The step records the artifact as a version-pinnable reference and only passes fields accepted by the environment.Reviews (3) · Last reviewed commit: "style(deploy_env): drop the comment on t..." · Reviewed by Greptile