Repository navigation
(feat) restore rate oracle config endpoints, scoped to bots - #235
Conversation
Replacing RateOracle with the in-house ticker pool (f067c37) also deleted the endpoints that set rate_oracle_source in conf_client.yml. The API no longer uses it, but every deployed bot still runs hummingbot's RateOracle from the conf_client.yml copied out of its credentials profile, so bots were stuck on the template's source. Adds GET /bot-orchestration/rate-oracle/sources and GET/PUT /bot-orchestration/rate-oracle/config, per account (account_name, default master_account). Updating master_account's global token also switches the API's own quote token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ly after persisting A blank global_token_name was persisted and installed as the API quote token, so every cross-rate lookup resolved to nothing (and kept doing so after a restart). And the live token was switched before conf_client.yml was written, so a failed write left the API on the new token while bots kept the old one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…GET /system/info
The Settings -> Server panel of a dashboard can say which server it is talking to but
not what that server runs: the hummingbot-api version, the hummingbot library version
behind it, and whether the image is the published one or something pinned on the box.
That last one is the question asked after every incident ("is this host actually on the
build we shipped?"), and today it is answered by ssh-ing in and reading `docker inspect`.
/system/info answers it over the API. It reports the API version, the hummingbot
version from importlib.metadata, and the API's own container as seen through the
already-mounted docker.sock: id, name, image reference, image id, RepoDigests digest,
and the compose project, working_dir and config_files labels. From those it derives
`pinned`: an image with no registry digest was built locally, and a config_files entry
whose name contains "override" means compose was brought up with a file that can
repoint `image:` (the tailscale overlay does not count -- it changes networking, not
the image). `pinned_reason` says which, and `override_file` names the file.
The route is read-only and never raises. Without docker.sock, with an unreachable
daemon, or when this process cannot place itself, it answers `docker_available: false`
with a null container and a null (not false) `pinned` -- unknown is not "unpinned" --
and still reports both versions, which is the part that works from source too.
The container is found the way a process finds itself from inside Docker: the hostname
is the short container id, with a fallback scan for the com.docker.compose.service
label for deployments that override `hostname:` or run the API from source.
VERSION moves from main.py to a new version.py so the router can read it -- main.py
imports every router, so importing it back would be circular. main.py re-exports it,
and the Docker buildx workflow's grep now reads version.py.
31e671d moved VERSION out of main.py so routers can read it without importing main, but the Dockerfile copies source files one by one and version.py was not on the list, so the built image would fail at startup on `from version import VERSION`. The build itself stays green -- nothing imports the app at build time -- which is why the miss is worth a line of its own here.
A dashboard's server panel can now say what a server runs, but not how it is tuned. The MARKET_DATA_* knobs are the ones an operator actually asks about after a slow ticker or a dropped subscription -- ticker_update_interval, ticker_max_age, the /ws/executors interval floor -- and today reading them means ssh-ing in and opening the .env. /system/info reports them alongside the versions, as this process resolved them, which is the honest answer: the file on disk may have been edited since the API started. They are read-only by construction and stay that way -- env_file is not mounted into the container, so the API cannot rewrite its own .env, and pretending otherwise would promise a change that silently disappears. Safe to report: MarketDataSettings holds intervals and timeouts and nothing else. A test pins that -- every value numeric, no key that reads like a credential -- so a field added later that does not belong in this payload fails here rather than leaking. Another pins that the tunables are still reported when the Docker daemon is unreachable, which is the same degrade path the versions already take. The mock container id in the test file gains a `noqa: mock` tag: it is 64 hex characters, which the wallet-private-key hook cannot tell from a key.
… client defaults The rate-oracle endpoints already read and write the slice of a credentials profile's conf_client.yml that deployed bots inherit. rate_limits_share_pct -- the share of each exchange's API rate limit one bot instance may spend -- is the other knob operators actually tune per profile, and it is the one they reach for when several bots share an account and start tripping the exchange's limiter. It was the last reason to open the file by hand. It goes here rather than behind an endpoint of its own on purpose. This router is the only writer of that file, and a second one would have to repeat the source validation and would silently skip the live-quote-token switch at the end of the PUT -- two writers of one file disagreeing about the side effects is the bug, not the duplication. The set written stays closed and small, so no caller can reach the mqtt_bridge or gateway credentials sharing the file. The bounds are hummingbot's own: gt=0, le=100, copied from ClientConfigMap so a value the bot would reject at startup cannot be persisted here -- a bad write breaks the next deploy, not the request that made it, which is the expensive way to find out. Reading degrades instead: the key is absent from every profile created before this commit, and a hand-edited file can hold anything, so an unreadable value answers with hummingbot's default rather than failing a GET that is only asking what bots will run with.
|
Tested this branch deployed rather than just unit-tested — built the image and ran the compose stack, and separately ran The feature works. The premise is demonstrated: I set Three issues below, none of which the existing tests catch — all three only show up against a real deployment. Issue 1 —
|
…d image Upgrading a hummingbot-api server means ssh-ing to the host and running `docker compose pull && up -d`. Condor's updater only knows how to do that in a local checkout, so every remote server is a manual job and servers drift -- which is how a box ends up months behind with nobody able to say so. Three routes, off the same /system prefix that already reports what a server runs: GET /system/upgrade/preflight can this server upgrade itself, and what would it cost POST /system/upgrade pull the image and hand the recreate to a helper GET /system/upgrade/status follow the run, including across the restart it causes A process cannot recreate its own container: `compose up` stops the old one, killing the process running compose halfway through, and the image has no compose plugin anyway. So the API pulls in-process -- what it already does for every other image -- and starts a one-shot docker:cli container with the socket and the compose working dir bound at the same host path, running the `docker compose -p <project> -f <files...> up -d --no-deps hummingbot-api` the operator would have typed. That helper outlives the API it replaces, and is also the run record: its exit code and log tail are what the *new* container reads on boot, so there is no state file and no table. The working dir is bound at the identical path because that is what makes ./bots, .env and the override paths inside those compose files resolve to the same things they resolved to when the stack was brought up. Everything here defaults to refusing, because the failure mode is a trading API server that does not come back. preflight starts blocked and clears that in exactly one place, once every fact it needs has been established; a fact it could not establish is treated the same as a fact that says no. An unreachable daemon, a container it cannot place, a stack with no compose labels, a registry digest it could not read, an executor count it could not take, a helper listing the daemon refused -- all of them are can_upgrade false with a reason, never a 500 and never an optimistic default. start re-runs the whole preflight rather than trusting the caller's earlier read, because between the dialog opening and the button being pressed an executor can start or another operator can begin a run, and it claims the run under the same lock it checked, so two callers cannot both pass and race two `compose up`s at one container. A pinned deployment is refused outright and named: an image built on the box, or a compose override file that can repoint `image:`. Removing an override or swapping a hand-built tag is a decision about the operator's own host -- moneymaker's pr226 pin is exactly this -- and silently rolling such a server onto the published build is worse than not upgrading. Those stay an SSH job, and the reason says which file. The consent is explicit and withheld by default: a restart closes every RUNNING executor as SYSTEM_CLEANUP and they are not restored, so acknowledge_executor_loss defaults to False and a body-less POST is refused with a 409 rather than served. Bot containers are separate and keep running, which preflight reports so the operator can see it. Both images are pulled before anything is stopped, the helper's included -- leaving that one to containers.run's implicit pull would put the failure after the recreate was already committed. A pull that fails leaves the server untouched and running, and reports the image that failed rather than the daemon's own string, which carries the socket URL and the negotiated API version. utils/compose.py is the three container/pinning helpers lifted out of routers/system.py verbatim, because the panel's "is this pinned" and this module's "refuse if pinned" have to be the same answer. A panel saying not pinned over a path that refuses is a confusing afternoon; a panel saying not pinned over a path that *proceeds* on a hand-built image is a trading API server replaced by a build nobody asked for. Out of scope and stated as such: switching tags or images, editing override files, rebuilding from source, upgrading postgres/emqx/gateway, and rollback.
… not 500 The app's RequestValidationError handler passed exc.errors() straight to JSONResponse; a validator raising ValueError puts the exception object in the error ctx, which is not JSON-serializable, so a blank global_token_name came back as a 500. Encode with jsonable_encoder, and move the handler to utils/validation_errors.py so the rate-oracle tests mount the real one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ublished image - own_container: also find itself by the container id in /proc/self/mountinfo (survives a custom hostname:); use the compose-service label fallback only from inside a container and only when exactly one container matches. - describe_container: only a digest in hummingbot/hummingbot-api counts, since the containerd image store gives local builds a RepoDigest of their own. - pinning: a tag other than latest, or a digest-pinned reference, is pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for testing this deployed. All three are fixed: Issue 1: Issue 2: blank Issue 3:
New tests reproduce your cases: the containerd 🤖 Generated with Claude Code |
|
Retested at 1. A failed upgrade leaves the API permanently stuck (confirms Greptile's P1)If the helper exits non-zero without replacing the API, The refusal text makes it worse: "Wait for it to finish; it is cleared when this API next starts." It has finished ( Steps to reproduce # with a server where preflight reports can_upgrade: true
# 1. break compose AFTER the container labels are stamped (preflight reads labels, not the file)
cat >> docker-compose.yml <<'EOF'
bogus-service:
image: this/image/does-not-exist:nope
depends_on:
- nonexistent-service
EOF
# 2. preflight still allows it
curl -s -u admin:admin localhost:8000/system/upgrade/preflight | jq '.can_upgrade' # true
# 3. start the upgrade; the helper fails and the API is never replaced
curl -s -u admin:admin -X POST localhost:8000/system/upgrade
docker ps -a --filter label=hummingbot-api.self-upgrade --format '{{.Names}} {{.Status}}'
# 4. the stuck state
curl -s -u admin:admin localhost:8000/system/upgrade/status | jq '{phase, exit_code, log_tail}'
curl -s -u admin:admin localhost:8000/system/upgrade/preflight | jq '.blocked_reason'
curl -s -o /dev/null -w '%{http_code}\n' -u admin:admin -X POST localhost:8000/system/upgradeObserved:
Note when reproducing: the refusal may first cite pinning rather than the helper, because the upgrade's pull moves the 2. hummingbot/hummingbot-api-client#27 no longer covers this PRThat PR is still at
Steps to reproduce # against a server running this branch
python - <<'PY'
import json, urllib.request, base64
vend = json.load(open("hummingbot-api-openapi.json"))
req = urllib.request.Request("http://127.0.0.1:8000/openapi.json")
req.add_header("Authorization", "Basic " + base64.b64encode(b"admin:admin").decode())
live = json.load(urllib.request.urlopen(req))
print(sorted(set(live["paths"]) - set(vend["paths"])))
PYLive PUT body accepts 3. Blank
|
…start A helper that exited without replacing the API was only collected by collect_on_boot(), which never runs because the API was not replaced: the run stayed 'recreating' forever and every later preflight refused on the exited helper. preflight() and status() now collect exited helpers too; collected from a live process the run is 'failed' whatever the exit code, since this process is the container the helper was meant to replace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ling Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for the retest. Status of the three: 1. Failed upgrade stuck at
2. hummingbot-api-client#27 is behind. The follow-up commits ( 3. Blank Suite: 901 passed. The 3 failures are the 🤖 Generated with Claude Code |
| @field_validator("global_token_name", "global_token_symbol") | ||
| @classmethod | ||
| def _reject_blank(cls, v: Optional[str], info) -> Optional[str]: | ||
| # A blank quote token would make every cross-rate lookup resolve to nothing, and a | ||
| # blank symbol renders every value in the bot's status without a unit. Omit the | ||
| # field to leave it unchanged. | ||
| if v is not None: | ||
| v = v.strip() | ||
| if not v: | ||
| raise ValueError(f"{info.field_name} must not be blank") |
There was a problem hiding this comment.
Extending this validator to global_token_symbol makes the read endpoint fail validation when an existing or manually edited conf_client.yml contains an empty symbol. _to_config passes that persisted value directly into this model, and the GET route does not handle the resulting validation error. This makes the account’s configuration endpoint return a server error instead of exposing or normalizing the configuration.
|
Retested at 1. A foreign upgrade helper is adopted as this API's own runConfirms Greptile's P1, with two consequences beyond it.
Beyond the case Greptile described:
Steps to reproduce # API idle, no upgrade ever started on it
curl -s -u admin:admin localhost:8000/system/upgrade/status | jq '{run_id, phase}'
# {"run_id": null, "phase": "idle"}
# plant an exited helper as another stack's upgrade would leave behind
docker run -d --name foreign-helper \
--label hummingbot-api.self-upgrade=FOREIGN_RUN_9999 \
--label hummingbot-api.self-upgrade.previous-digest=hummingbot/hummingbot-api@sha256:deadbeef \
alpine:latest sh -c 'echo "some other stack failed"; exit 7'
curl -s -o /dev/null -u admin:admin localhost:8000/system/upgrade/preflight # collects
curl -s -u admin:admin localhost:8000/system/upgrade/status | jq
docker ps -a --filter label=hummingbot-api.self-upgradeObserved: {
"run_id": "FOREIGN_RUN_9999",
"phase": "failed",
"exit_code": 7,
"previous_digest": "hummingbot/hummingbot-api@sha256:deadbeef",
"log_tail": ["some other stack failed"],
"detail": "The recreate did not replace this API (exit code 7); it is still running the previous image..."
}and the foreign helper is gone from Matching the run id before adopting a record, and scoping the helper lookup to this container's 2. A hand-edited
|
| persisted state | GET | PUT (source only) |
|---|---|---|
global_token_symbol: '' |
500 | 500 |
global_token is a string |
500 | 500 |
global_token is a list |
500 | 500 |
rate_oracle_source is a string |
500 | 500 |
rate_oracle_source.name: null |
500 | 200 |
global_token_symbol is a number |
500 | 500 |
| file is invalid YAML | 500 | 500 |
| file is a YAML list | 500 | 500 |
global_token_name: '' |
200 | 200 |
rate_limits_share_pct: 150 or 'abc' |
200 | 200 |
The rows that pass are the defensively written ones — global_token_name=... or "USDT", and _share_pct, whose docstring already states the principle: "a hand-edited file can hold anything, so an unreadable value reads as the default rather than failing the whole GET." The rest of _to_config does not follow it.
Worth noting the API cannot produce these states itself — every malformed write is rejected with a 422 and the file stays readable, so this needs a hand edit, a legacy file or a migration.
Steps to reproduce
P=bots/credentials/master_account/conf_client.yml
cp "$P" /tmp/conf_client.bak
python3 - <<'PY'
import yaml
p = "bots/credentials/master_account/conf_client.yml"
d = yaml.safe_load(open(p))
d["global_token"]["global_token_symbol"] = "" # or: d["global_token"] = "nonsense"
yaml.safe_dump(d, open(p, "w"))
PY
curl -s -o /dev/null -w 'GET %{http_code}\n' -u admin:admin \
localhost:8000/bot-orchestration/rate-oracle/config
curl -s -o /dev/null -w 'PUT %{http_code}\n' -u admin:admin -X PUT \
-H 'Content-Type: application/json' -d '{"rate_oracle_source":{"name":"binance"}}' \
localhost:8000/bot-orchestration/rate-oracle/config
cp /tmp/conf_client.bak "$P"Observed: GET 500, PUT 500. Same for each shape in the table above.
🤖 Generated with Claude Code
| if expected_run_id is not None and run_id != expected_run_id: | ||
| logger.info(f"Self-upgrade helper {helper.name} is not the run this API is tracking; leaving it.") | ||
| continue |
There was a problem hiding this comment.
On boot, the service does not have an expected run ID, so every exited helper from the same Compose project passes this check. If multiple uncollected helpers have accumulated, the loop adopts and removes all of them while repeatedly overwriting _run in Docker's list order. A stale helper can therefore replace the latest upgrade result, permanently deleting the newer record and making /system/upgrade/status report the wrong run. Select one authoritative helper on boot, such as the newest, without allowing older helpers to replace its status.
|
Thanks for the retest. Status of the two: 1. A foreign upgrade helper is adopted as this API's own run. Fixed in 9292dec.
2. A hand-edited
Both are covered by regression tests; full suite run locally: 70 passed (42 in The hummingbot-api-client#27 follow-up you flagged is in that repo and out of scope here. |
3 participants
@david-hummingbot could you review this one?
Why
f067c37 replaced the API's
RateOraclewith the in-house ticker pool and deletedrouters/rate_oracle.py, which also removed the only way to changerate_oracle_sourcethrough the API. The API itself no longer needs it, but deployed bots still run hummingbot'sRateOraclefrom theconf_client.ymlcopied out of their credentials profile at deploy time. Without these endpoints every bot always starts with the template's source (gate_io).What
Config-only endpoints under bot orchestration, so it's explicit they configure bots, not the API's pricing:
GET /bot-orchestration/rate-oracle/sources: valid source namesGET /bot-orchestration/rate-oracle/config?account_name=master_account: current source and global tokenPUT /bot-orchestration/rate-oracle/config?account_name=...: validates the source and saves it tobots/credentials/<account>/conf_client.ymlIt works per account (credentials profile), since that is what each bot copies. Setting
global_token_nameonmaster_accountalso switches the API's own quote token live (newMarketDataService.quote_tokensetter).Notes
master_account'sconf_client.ymlonly once, when created, so existing accounts must be updated individually.conf_client.ymlare lost (same as the removed endpoint).Also:
GET /system/infoWhy: a dashboard's Settings → Server panel can say which server it is talking to, but not what that server runs. Which
hummingbot-apiversion, whichhummingbotlibrary behind it, and — the question asked after every incident — whether the image is the published one or something pinned on that box. Today that is answered by ssh-ing in and readingdocker inspect.What it returns (added to the existing
routers/system.py, same auth as the rest):api_version— the API'sVERSIONhummingbot_version—importlib.metadata.version("hummingbot"),nullif not installeddocker_available— whether the daemon could be reachedcontainer— the API's own container through the already-mounteddocker.sock:id,name,image,image_id,digest(fromRepoDigests),compose_project,compose_working_dir,compose_config_filespinned/pinned_reason/override_file—pinnedis true when the image has no registry digest (built locally) or when the composeconfig_fileslabel lists an override file, which is then named.docker-compose.tailscale.ymldoes not count: it repoints networking, not the image.It identifies itself the way a process does from inside Docker — the hostname is the short container id — falling back to a scan for the
com.docker.compose.servicelabel when the deployment overrideshostname:or runs the API from source.Read-only, and it never raises. Without
docker.sock, with an unreachable daemon, or when the process cannot place itself, it answersdocker_available: falsewith anullcontainer and anull(notfalse)pinned— unknown is not "unpinned" — and still reports both versions.VERSIONmoves frommain.pyinto a new one-lineversion.pyso a router can read it without importingmain.pyback (main imports every router).main.pyre-exports it, and the Docker buildx workflow'sgrepnow readsversion.py.Tests
pytest test/test_bot_rate_oracle_config.py: 8 passed (read, persist without clobbering other keys, per-account isolation, quote token sync, invalid source → 400, unknown/unsafe account → 404/400).pytest test/test_system_info.py: 8 passed with the docker client mocked (versions + digest + compose labels, locally built image → pinned, override file → pinned and named, tailscale overlay → not pinned, hostname miss → compose-service-label fallback, unreachable daemon / unidentifiable container / no docker client →docker_available: falsewith the versions still reported).🤖 Generated with Claude Code