fix(update): install keel_broker_alpaca on self-update (#425) - #504
Merged
Conversation
The equities profile (config.paper-equities.yaml, broker: name: alpaca, #386) deploys keel_broker_alpaca beside the four base wheels, so the self-update set must include it: an updater that moved the rest and left the adapter behind failed its own verify step (keel versions: PARTIAL INSTALL) and rolled the whole deployment back -- every self-update on such a box failed. The pin is an exact-tuple assertion, stated by name: the set stays a named list (never Release/*.whl, never config-derived), and the excluded venues are pinned as excluded -- keel_broker_fake (registers a fake venue entry point), keel_broker_robinhood (an Ed25519 stack for an adapter nothing constructs) and keel_broker_kraken (a stub, #313, whose every data method raises). The verify step is pinned to demand keel-broker-alpaca present at the target, the run-order pin derives its download count from the constant instead of a hardcoded four, and the runbook self-update pin demands the section name alpaca with the other prefixes.
PRODUCTION_WHEEL_PREFIXES gains keel_broker_alpaca: the equities profile (config.paper-equities.yaml, broker: name: alpaca, #386) deploys the adapter beside the four base wheels, and an update that moved the rest while leaving it behind failed the verify step (keel versions: PARTIAL INSTALL) and rolled the WHOLE deployment back to the previous version -- every self-update on such a deployment failed, by construction, from v0.10.0 (the first release shipping the adapter next to a config that selects it). The set stays stated by name (the issue's 'awkward part'), not derived from the running config or the installed set: naming keeps the plan knowable before anything reads a deployment's config, and the cost -- the adapter present on a Coinbase-only box -- is one unused module whose single dependency, requests, already rides every deployment transitively via the Coinbase SDK. The deliberate exclusions are re-stated and extended: keel_broker_fake (registers a fake venue entry point), keel_broker_robinhood (an Ed25519 stack for an adapter nothing constructs) and keel_broker_kraken (a stub, #313, whose every data method raises -- it must never ride an update either). Every 'four wheels' wording that became false moves with it: the module docstring, the select/verify error text, the gate and step lines, and the operator docs (RELEASING.md's assets table gains the alpaca row the issue called stale, plus kraken in the not-deployed row; the runbook's manual procedure names the fifth wheel path and its self-update section says five).
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 was wrong (confirmed, not assumed)
keel updateon an equities deployment (config.paper-equities.yaml,broker: name: alpaca,keel-broker-alpacainstalled in the venv) fails at verification, after the wheels are in, then reverts the whole deployment — exactly the "failure AFTER a finished install" path the issue suspected, not a silent mixed install and not an overrideable warning. The trace (line numbers frommain):plan_update→select_production_wheelspicks exactly the four prefixes — the constant has nokeel_broker_alpaca(keel/commands/update.py:87-92), the selection at:363-397, wired at:426. The plan is offered; nothing refuses.run_updatebacks up the DBs, downloads those four wheels (:767-779) and installs them (:799-800,installed = True).keel_core/keel_broker_api/keel_broker_coinbase/keel_tradermove to the new version;keel-broker-alpacastays at the old one — nothing selects its wheel._verify_versions(:632-659) runs the NEW build'skeel versions, which lists every keel distribution in the venv: the alpaca row is old (:649-651→ "keel-broker-alpaca is at X, not Y"), andkeel versionsitself exits 1 withPARTIAL INSTALL(keel/version.py:258, exit atkeel/commands/versions.py:83) →UpdateError("verify failed: ...")at:659.installedis True, so the post-install failure branch runs (:809,:839-877): the previous wheels are re-installed fromRelease/best-effort (:840-845),rolled_back=True, and the error points at the manual runbook procedure. The new wheels stay inRelease/; the backups stay.So every self-update on an equities deployment fails and reverts, from
v0.10.0— the first release publishingkeel_broker_alpacabeside a config that selects it.The fix
keel_broker_alpacajoinsPRODUCTION_WHEEL_PREFIXES(now five). Selection, install, the superseded set, cleanup and the verify floor all derive from the one constant, so the adapter moves with the rest andkeel versions' all-must-agree invariant holds by construction. The plan/gate/manual-recovery wording that said "four" follows.The "awkward part", addressed
The issue weighs three shapes and leans to (2) (install venue wheels already present in the venv, keeping four as the floor). This PR takes (3) (the named five-element constant), deliberately:
requestsdependency forced on it" — is void in fact:requestsalready rides every deployment transitively via the Coinbase SDK (coinbase-advanced-pydepends on it;uv.lock). What (3) actually puts on a Coinbase-only box is one small unused adapter module.broker: name:) couples the updater to config parsing — the plan could no longer state its wheel set without first reading a deployment's config.keel_broker_fake(registers afakevenue entry point),keel_broker_robinhood(an Ed25519 stack for an adapter nothing constructs) andkeel_broker_kraken(the just-merged stub, packages/keel-broker-kraken: a stub adapter — port implemented, every method raising NotImplementedError #313 — every data/market method raises; it must never ride an update). The#:docstring on the constant re-states all of it, including why the set is stated by name rather than derived; if a third real venue ever ships, (2) is worth revisiting.Also stale (the issue's last section)
docs/RELEASING.md's "Release assets" table gains akeel_broker_alpaca-*row (a deployment installs it beside the four;keel updatemoves all five) andkeel_broker_kraken-*joins the published-but-not-deployed row. The runbook follows: the manual procedure names the fifth wheel path (with the why), thekeel versionssample shows five distributions, and the self-update section says five and names the kraken exclusion.Tests (TDD: test commit first, then the fix)
keel_broker_alpacaIN;keel_broker_fake/keel_broker_robinhood/keel_broker_krakenOUT.keel-broker-alpacapresent at the target (the pre-fix "healthy" four-distribution output now fails verify); the run-order pin derives its download count from the constant instead of a hardcoded four; the runbook self-update pin demands the section name alpaca.["download"] * 4and thelen(...) == 4in the zero-byte-download test were the only sites; both now derive).Gates
uv run pytest -q— 4348 passed, 3 skippeduv run ruff check keel tests packages— cleanuv run mypy— clean (343 files)uv run ruff format --checkon the touched files flags only hunks that exist identically onmain(CI gatesruff check, notformat); this diff adds none — verified by hunk-for-hunk comparison against theorigin/mainblobs.Closes #425