feat: select the hosted region via a Region enum - #6
Conversation
…ault URL The hosted service runs as independent per-region deployments (au / sg), each with its own database; a workspace key only authenticates against the region that issued it. WorkspaceClient hardcoded a single _DEFAULT_BASE_URL, which is not a valid per-region host and silently sent traffic to one region regardless of where the key was minted -> 401. Add a Region enum (au/sg -> regional base URL) and a keyword-only `region` argument. WorkspaceClient now takes `region` OR an explicit `base_url` (escape hatch for self-hosted / a not-yet-listed region), not both; with neither it falls back to XAGENT_BASE_URL and otherwise raises rather than guessing a host. The hardcoded default is removed. The base-url error hint is made overridable via a _BASE_URL_HINT class attribute (mirroring _API_KEY_FIELD) so the workspace client points the caller at `region=`; other clients keep the generic hint. Region is exported from xagent_sdk.cloud (not the top level).
Rework the WorkspaceClient quickstart to select a region instead of a hardcoded URL, and reuse that region's host for the AgentClient that runs the minted agent. Add a public Region.base_url property so callers (and the example) can get the host without importing a private mapping; the client resolves region -> base_url through the same property.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
rogercloud
left a comment
There was a problem hiding this comment.
Design verdict: sound ✅
This addresses the root cause rather than a symptom. The hosted service is genuinely per-region (separate DBs, region-scoped keys), so the single hardcoded _DEFAULT_BASE_URL was structurally wrong. The fix:
- Introduces a declarative
Regionenum mapping region codes → API hosts. - Removes the default and fails fast (
regionXORbase_url, elseXAGENT_BASE_URL, else raise) rather than guessing a host — silently routing to one region is exactly the failure mode being fixed. - Reuses the existing
_API_KEY_FIELD-style override pattern via the new_BASE_URL_HINTClassVar, fitting the_BaseClientmodel cleanly.
Library code (region.py, workspace_client.py, _base.py) is correct on every resolution path (region wins over env; empty string still fails fast via _resolve; conflict raises). Unit tests are thorough. Locally: mypy strict + ruff clean, full unit suite green.
Findings
The library and unit tests were migrated thoroughly, but the e2e cloud suite was not — it still encodes the old, removed default. These are opt-in tests (skipped in CI), so none block CI, but they directly concern the shipped feature.
1. [Major] e2e smoke test hardcodes the host this PR declares invalid
python/tests/e2e/test_cloud_smoke.py:33
return os.environ.get("XAGENT_BASE_URL") or "https://cloud.xagent.run"The PR description states https://cloud.xagent.run "is not a valid per-region API host" — that's the bug being removed from the library. The smoke test for this very feature still falls back to it, so running e2e without XAGENT_BASE_URL silently routes runtime traffic to the dead host. Resolve from the Region used (e.g. region.base_url, mirroring the README flow) or require explicit config rather than defaulting to a known-invalid URL.
2. [Minor] test_bad_workspace_key_unauthorized now raises ValueError, not InvalidAPIKey
python/tests/e2e/test_cloud_smoke.py:68-76
base_url = os.environ.get("XAGENT_BASE_URL") # may be None
with (
WorkspaceClient(workspace_key="xag_workspace_bad_key", base_url=base_url) as c,
pytest.raises(InvalidAPIKey),
):With the default removed, base_url=None raises ValueError in the constructor, so the pytest.raises(InvalidAPIKey) path is never exercised when XAGENT_BASE_URL is unset. Pre-existing code, but directly broken by this change — pass a region/base_url.
3. [Minor] Stale e2e docstrings referencing the removed hosted default
python/tests/e2e/conftest.py:98-101— theworkspace_clientfixture says base_url "falls back toXAGENT_BASE_URLand then the hosted default … one hitting the hosted service sets only the key." No longer true: key-only now raisesValueError. The fixture itself (base_url=base_url, possiblyNone) shares the latent issue in finding #2.python/tests/e2e/test_cloud_smoke.py:7— "XAGENT_BASE_URL optional; defaults to the hosted endpoint" is no longer accurate.
Summary
Solid, well-scoped change with a correct design and excellent unit coverage. The only gap is that the e2e cloud surface (smoke test + fixture) wasn't migrated alongside the library — it still hardcodes the invalid cloud.xagent.run and documents a key-only workflow that now fails fast. Recommend addressing #1 before merge; #2 and #3 are cleanup that can ride along.
Problem
The hosted service runs as independent per-region deployments (
au/sg), each with its own database; a workspace key only authenticatesagainst the region that issued it.
WorkspaceClienthardcoded a single_DEFAULT_BASE_URL = "https://cloud.xagent.run", which is not a validper-region API host and silently sent traffic to one host regardless of
where the key was minted — producing
401 invalid_api_key. A singleglobal default is also exactly what the multi-region backend forbids
(API traffic must not be proxied to a default region).
Change
Regionenum (AU/SG→ regional base URL) with a publicRegion.base_urlaccessor.WorkspaceClienttakes a keyword-onlyregionor an explicitbase_url(escape hatch for self-hosted / a not-yet-listed region),not both. With neither it falls back to
XAGENT_BASE_URL; otherwise itraises rather than guessing a host. The hardcoded default is removed.
_BASE_URL_HINT, mirroring_API_KEY_FIELD) so the workspace client points the caller atregion=; other clients keep the generic hint.Compatibility
WorkspaceClientno longer defaultsbase_url, andregion/base_urlare keyword-only — callers must now pass a
regionorbase_url(or setXAGENT_BASE_URL). Versioned 0.3.1.Regionis exported fromxagent_sdk.cloudonly; the top-level surface is unchanged.Validation
Region.{SG,AU}.base_url,region→URL resolution,region+base_urlconflict raises, neither + no env raises (no hosted default),base_url/XAGENT_BASE_URLescape hatch still works,cloud.__all__ == {WorkspaceClient, Region}, neither leaked at the toplevel.