Skip to content

fix(packaging): add keyring to [api] extra so gaia api + email wheel starts#1620

Merged
itomek merged 6 commits into
mainfrom
fix/issue-1617-keyring-api-extra
Jun 12, 2026
Merged

fix(packaging): add keyring to [api] extra so gaia api + email wheel starts#1620
itomek merged 6 commits into
mainfrom
fix/issue-1617-keyring-api-extra

Conversation

@itomek

@itomek itomek commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Why this matters

The documented Milestone-40 partner recipe — pip install 'amd-gaia[api]' gaia-agent-email, then gaia api — crashed at server startup with ModuleNotFoundError: No module named 'keyring', before serving a single request. gaia api auto-mounts the email wheel's REST router, and that import chain reaches a module-level import keyring in gaia.connectors.store (also hit at request time via connected_mailbox_providers()) — but keyring was declared only in the [ui]/[dev] extras, never [api]. So the one install combo the partner path depends on couldn't even boot; 0.20.1 only masks it because its server doesn't mount the email router yet.

After this change [api] carries keyring (same pin as [ui]/[dev]), so the documented install starts and serves local LLM triage with zero manual installs. Needs to land before the v0.21.0 tag. A packaging guard (tests/unit/test_api_extras.py, mirroring test_ui_extras.py) fails if [api] ever drops keyring again.

Test plan

  • Guard test is red on main (keyring absent from [api]), green with the fix.
  • pytest tests/unit/test_api_extras.py tests/unit/test_packaging.py tests/unit/test_ui_extras.py → 13 passed; python util/lint.py --all → clean.
  • Real-world, Linux + Radeon dGPU, local Lemonade (Gemma-4-E4B), fresh venvs via the documented git+ recipe:
    • Before (main): from gaia.api.openai_server import appModuleNotFoundError: No module named 'keyring' at store.py:38 (keyring not installed).
    • After (this branch): keyring 25.7.0 pulled by [api] → import clean → uvicorn boots → POST /v1/email/triage200 with LLM triage (category actionable, summary echoing the planted $4,820 / INV-90871 / Thursday 3pm facts, action item + draft reply).

Out of scope: AC3 (partner guide stating the exact install line) is tracked by #1597, and depends on #1601 publishing the gaia-agent-email wheel.

Closes #1617.
Related: #1602, #1590, #1597, #915, #1179.

Tomasz Iniewicz added 2 commits June 11, 2026 19:45
Adds a packaging regression test that fails when setup.py[api] is missing
keyring — the module reached via openai_server -> email_router ->
gaia.connectors.api -> gaia.connectors.store at load time and via
connected_mailbox_providers() at request time. Mirrors the structure of
test_ui_extras.py (#845) exactly: parses setup.py line-by-line to avoid
bracket-in-comment false matches.
…nt-email starts (#1617)

gaia api auto-mounts the gaia-agent-email REST router when that wheel is
importable; its import chain (openai_server -> email_router ->
gaia.connectors.api -> gaia.connectors.store) does `import keyring` at
module load AND inside connected_mailbox_providers() at request time.
keyring was only in [ui]/[dev], so `pip install 'amd-gaia[api]'
gaia-agent-email` crashed on startup. Adds the same pin as [ui]/[dev]
(>=24.0.0,<26.0.0).
@github-actions github-actions Bot added dependencies Dependency updates tests Test changes labels Jun 11, 2026
@itomek itomek marked this pull request as ready for review June 11, 2026 23:58
@itomek itomek requested a review from kovtcharov-amd as a code owner June 11, 2026 23:58
@github-actions

Copy link
Copy Markdown
Contributor

Review: fix(packaging): add keyring to [api] extra (#1620)

Approve. This is a correctly-scoped, well-documented one-line packaging fix backed by a TDD-style guard test, and I verified every claim in the description against the actual code. Safe to land before v0.21.0.

Summary

gaia api mounts the gaia-agent-email REST router (src/gaia/api/openai_server.py:111-113), and that import chain bottoms out at a module-level import keyring in src/gaia/connectors/store.py:38 — hit again at request time via connected_mailbox_providers() (src/gaia/connectors/api.py:547). keyring was declared in [ui]/[dev] but not [api], so the documented pip install 'amd-gaia[api]' gaia-agent-email recipe crashed on startup. The fix adds keyring>=24.0.0,<26.0.0 to [api] — the exact same pin as [ui]/[dev] — and a guard test mirroring the established test_ui_extras.py (#845) pattern.

Verified locally:

  • The two PR commits (test 859048da, then fix f5e482af) are not yet on origin/main — no duplicate.
  • Guard test parses [api] correctly (['fastapi…', 'uvicorn…', 'python-multipart…', 'keyring>=24.0.0,<26.0.0']) and passes; it skips comment-only lines so the bracket in the new comment block doesn't confuse the parser.
  • The pin matches [ui] (setup.py:139) and [dev] (setup.py:200) exactly.

Issues

🟢 Possible broader gap — is gaia connectors (a base command) also exposed? (src/gaia/connectors/cli.py:244,407)
Not blocking, and out of scope for this PR — flagging for a follow-up. gaia connectors is registered on the base CLI (no extra gate), and its handlers lazily from gaia.connectors.store import …, which triggers the same module-level import keyring. So a plain pip install amd-gaia (no extras) running e.g. the peek_connection/save_provider_credentials paths would hit the identical ModuleNotFoundError. The lazy import means only those subcommands fail rather than all of gaia connectors, which is why it hasn't surfaced. Worth deciding separately whether keyring should be a base dependency rather than living in three extras — but that's a bigger call and shouldn't hold up the documented [api] partner recipe this PR fixes.

Strengths

  • TDD ordering done right — the red-on-main guard test is its own commit landing before the fix, so the regression is provably reproduced. Commits are logically clean (one test, one fix).
  • Strong pattern reusetest_api_extras.py mirrors test_ui_extras.py line-for-line (same line-walking parser that tolerates brackets in comments, same "packaging assertion, not runtime import" framing), so it runs in the unit venv without installing [api].
  • Comment earns its place — the setup.py block explains the non-obvious why (the import chain a future editor can't see from the dependency line) in a few tight lines, exactly the kind of WHY comment CLAUDE.md asks for.

Verdict

Approve — no blocking issues. The 🟢 note above is a follow-up question, not a change request for this PR.

@itomek itomek enabled auto-merge June 12, 2026 00:11
Tomasz Iniewicz added 2 commits June 11, 2026 21:21
…o-get keyring (#1617)

gaia-agent-email declared a bare `amd-gaia>=0.20.0` dependency, so a consuming app's `pip install gaia-agent-email` could resolve core without the [api] extra — leaving the REST-server deps (fastapi/uvicorn) and keyring absent and `gaia api` crashing. Depend on `amd-gaia[api]` so those come along automatically; closes the "consumer forgot [api]" half of #1617. Guard test added. Floor stays >=0.20.0 (core is 0.20.1); raise to >=0.21.0 once that release ships keyring-in-[api].
… guard (#1617)

test_every_wheel_declares_amd_gaia_dependency did a naive `'amd-gaia>=' in text` check; the email wheel now declares `amd-gaia[api]>=0.20.0` (so consumers auto-get the REST-server deps + keyring), which the literal substring missed. Match `amd-gaia(\[extras\])?>=` instead — the invariant (every wheel pins amd-gaia with a >= floor) still holds.
@itomek

itomek commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

CI fix pushed (478d482). The unit-test failures (py3.10/3.11/3.12 + macOS smoke) were all the same test — test_agent_pypi_publish.py::test_every_wheel_declares_amd_gaia_dependency, which did a literal 'amd-gaia>=' in pyproject check that the new amd-gaia[api]>=0.20.0 form misses. Updated it to match amd-gaia(\[extras\])?>=; the invariant (every wheel pins amd-gaia with a >= floor) still holds. Verified locally + util/lint.py --all clean.

The Lemonade Embeddings API and CLI Full Integration failures are external infra, not this change — PyTorch mirror 503 during env setup, and huggingface.co 429 on the model pull. They should clear on re-run.

Re: the 🟢 follow-up — agreed. gaia connectors (a base command) lazily reaches the same import keyring via gaia.connectors.store, so a bare pip install amd-gaia running e.g. peek_connection/save_provider_credentials hits the identical error on those subcommands. Whether keyring should be promoted to a base dependency (vs. living in three extras) is a deliberately separate, bigger call I kept out of this PR to keep the partner-recipe fix tight. Happy to file a follow-up issue to track it.

@itomek itomek added this pull request to the merge queue Jun 12, 2026
Merged via the queue into main with commit 8421410 Jun 12, 2026
69 checks passed
@itomek itomek deleted the fix/issue-1617-keyring-api-extra branch June 12, 2026 02:30
pull Bot pushed a commit to bhardwajRahul/gaia that referenced this pull request Jun 12, 2026
amd#1622)

A bare `pip install amd-gaia` (no extras) ships `gaia connectors` as a
base command, but `keyring` lives only in the `[ui]`/`[api]`/`[dev]`
extras — so `gaia connectors list`/`status` and the provider-credential
paths crashed with a raw `ModuleNotFoundError: No module named
'keyring'` that named neither the cause nor the fix. Now those
subcommands fail loudly with an actionable `ConnectorsError` — `gaia
connectors needs the 'keyring' package … pip install keyring (or pip
install "amd-gaia[ui]")` — which the CLI prints to stderr instead of a
traceback.

This is the base-CLI half of the gap amd#1617/amd#1620 fixed for the `[api]`
partner recipe. It implements the issue's Option 3 (guarded import) —
the part the issue itself calls "worth doing regardless." The larger
base-dep-vs-`[connectors]`-extra packaging decision (Options 1/2) is
**left to a maintainer** (flagged for @kovtcharov-amd in the issue);
this guard holds regardless of where `keyring` ultimately lands, and on
its own satisfies the issue's acceptance branch "*fails with an
actionable error naming the exact install command.*"

Refs amd#1621

## Test plan

- [ ] `python -m pytest tests/unit/connectors/test_keyring_guard.py -x`
passes (new regression guard: import without `keyring` →
`ConnectorsError` naming `pip install`; with `keyring` → real module
re-exported)
- [ ] `python -m pytest tests/unit/connectors/ -x` passes (no regression
in store/mcp_server keyring usage)
- [ ] `python util/lint.py --all` passes
- [ ] Manual: in a venv with **no** `keyring` installed, `gaia
connectors list` prints `Connectors error: gaia connectors needs the
'keyring' package … pip install keyring …` and exits non-zero (instead
of a `ModuleNotFoundError` traceback)

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Tomasz Iniewicz <itomek@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(packaging): gaia api crashes at import when gaia-agent-email is installed — keyring missing from the [api] extra

2 participants