Skip to content

Collapse the duplicated blank-API-key check onto requireUsableApiKey #1897

Description

@jasperblues

Goal

One rule for "a blank API key counts as absent", in one place, across every BYOK entry point.

Why it matters

A key is blank rather than absent more often than it looks — Compose passes OPENAI_API_KEY=${OPENAI_API_KEY:-}, so in a container the variable is routinely set-but-empty. Two copies of the rule is two places for it to drift, and drift here means the same deployment gets an actionable error for its chat key and an opaque provider error for its embedding key.

Current state

Both copies land via PRs that were deliberately kept independent of each other:

The duplication was the cheaper trade at the time: sharing the helper would have stacked the two branches for four lines. The two changes merge cleanly and the merged tree compiles and passes, so this is tidy-up, not a defect.

What to do

Once both #1890 and #1892 have merged:

  1. Replace the guard in buildValidatedEmbeddingService with requireUsableApiKey(apiKey).
  2. Delete BLANK_EMBEDDING_KEY_MESSAGE.
  3. Point the two tests in OpenAiCompatibleModelFactoryByokEmbeddingTest at BLANK_API_KEY_MESSAGE.

The comment on BLANK_EMBEDDING_KEY_MESSAGE already says this, so the code will tell whoever gets there first.

Done when

grep -r "blank" embabel-agent-openai finds no second copy of the message, and the embedding path throws the same InvalidApiKeyException message as the LLM path for the same input.

Blocked on

#1890 and #1892.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions