refactor(config): promote helm-values generation to agentex.config.helm_values#402
Draft
max-parke-scale wants to merge 1 commit into
Draft
Conversation
…lm_values The manifest+environments.yaml -> helm-values mapping in agentex.lib.cli.handlers.deploy_handlers depends only on the stdlib and the agentex.config models, but living in the heavy ADK forced server-side deployers (egp-api-backend) to fork it — and the forks have drifted. Promote the pure mapping to agentex.config.helm_values (slim-safe, same contract as the #396 config-models promotion) and parameterize the consumer differences: repository/image_tag as explicit args, acp_module for pre-resolved ACP modules (filesystem resolution stays in agentex.lib.cli.utils.path_utils). The CLI wrapper keeps its signature, DeploymentError contract, conditional module resolution, and all current policy defaults — behavior-preserving except two before/after-merge debug log lines that no longer fire. Part of AGX1-357. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9a8cf39 to
552e880
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
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.
What
Promotes the manifest + environments.yaml → helm-values mapping from
agentex.lib.cli.handlers.deploy_handlersinto a new slim-safe module,agentex.config.helm_values— the same treatment #396 gave the config models. The mapping depends only on the stdlib and theagentex.configmodels, but living in the heavy ADK forced server-side deployers (egp-api-backend's Temporal deploy workflow) to fork it, and the forks have drifted.Step 1 of the plan in AGX1-357 (later steps move the policy defaults into the
agentex-agentchart; this PR deliberately keeps them).How the consumer differences are absorbed
repository/image_tagare explicit kwargs — the CLI resolvesInputDeployOverridesfalling back to the manifest image before calling; server-side deployers pass their freshly-built image.acp_moduleis an optional kwarg — the CLI passes the filesystem-resolved module (resolution stays inagentex.lib.cli.utils.path_utils, which needs the agent source tree); the default is a pure-string derivation fromlocal_development.paths.acp.ValueError; the CLI wrapper keeps its signature and maps toDeploymentError.auth_utils._encode_principal_context_from_env_confignow delegates to the promotedencode_principal_context(single source for the AUTH_PRINCIPAL_B64 encoding).deploy_handlersre-exports the historical names (TEMPORAL_WORKER_KEY,convert_env_vars_dict_to_list,_deep_merge,add_acp_command_to_helm_values,merge_deployment_configs) so existing imports keep working — pinned by tests, mirroringtest_config_shims.py.Also:
BuildContext.dockerignorenow declares its default asdefault=None— pyright doesn't recognize the positional form (the repo lint gate flagged the new tests'BuildContext(...)constructions), and every other optional field in these modules already uses the keyword.Behavior
Output is unchanged for all callers: same values dict, same defaults (autoscaling, pullPolicy, command injection), same conditional module resolution when
helm_overridesprovidescommand. The only observable delta is that two before/after-mergelogger.infodumps of the full helm values no longer fire mid-merge (the final "Deploying with the following helm values" log remains); those dumps include env-var values, so they were also dubious to emit from a server.Slim-safety verified:
scripts/check-slim-depspasses, and a differential import check showsagentex.config.helm_valuespulls in exactly whatagentex.config.agent_manifestalready does (noagentex.lib, no yaml/temporalio).Tests
tests/test_helm_values.py— pins the values contract (skeleton, env precedence, credentials→secretEnvVars, temporal blocks, principal encoding, pull secrets, deep-merge, error cases). The chart contract previously had no tests in this repo.tests/lib/cli/test_deploy_handlers.py— wrapper parity with the promoted core, override resolution,DeploymentErrormapping, fallback + skip behavior for module resolution, back-compat re-exports.tests/lib/cli/+tests/test_config_shims.py; ruff clean.🧑💻🤖 — posted via Claude Code