Skip to content

chore: use new model runner syntax#517

Merged
lionello merged 3 commits into
mainfrom
lio/new-model-runner
Jul 22, 2026
Merged

chore: use new model runner syntax#517
lionello merged 3 commits into
mainfrom
lio/new-model-runner

Conversation

@lionello

@lionello lionello commented Nov 14, 2025

Copy link
Copy Markdown
Member

Cf. https://docs.docker.com/ai/compose/models-and-compose/

Samples Checklist

✅ All good!

@lionello
lionello had a problem deploying to deploy-changed-samples November 14, 2025 22:05 — with GitHub Actions Failure
@lionello
lionello force-pushed the lio/new-model-runner branch from 82c5d5a to f43d2bf Compare November 25, 2025 04:41
@lionello
lionello had a problem deploying to deploy-changed-samples November 25, 2025 04:42 — with GitHub Actions Failure
@lionello
lionello force-pushed the lio/new-model-runner branch from f43d2bf to b841033 Compare May 25, 2026 19:34
@lionello
lionello had a problem deploying to deploy-changed-samples May 25, 2026 19:34 — with GitHub Actions Failure
@lionello lionello linked an issue Jun 23, 2026 that may be closed by this pull request
@defangdevs

Copy link
Copy Markdown
Contributor

Rebased this branch onto current main (3ea9bfc), but defangdevs has no write access to this repo, so the rebased branch is parked at defangdevs/samples:lio/new-model-runner (commit 75acd1e). To update this PR:

git fetch https://github.com/defangdevs/samples.git lio/new-model-runner
git push --force-with-lease origin FETCH_HEAD:refs/heads/lio/new-model-runner

Conflict resolution: both mastra-extended hunks were dropped as superseded — main already converted that sample to the top-level models: syntax (cc5225a) and then switched it to portable aliases without x-defang-llm (3cd3c98), and the dev model on main is now gemma3 rather than the qwen2.5 this branch carried. The rebased commit touches only the two crewai files.

Pre-existing issues in the remaining crewai diff (in the PR as originally authored — I didn't change content during the rebase):

  1. worker.depends_on still lists chat and embedding, which this PR removes as services — invalid compose.
  2. Neither app nor worker is attached to the new top-level models (no per-service models: block), so CHAT_URL / CHAT_MODEL / EMBEDDING_URL / EMBEDDING_MODEL — which app/core/ai_clients.py and custom_llm.py read — are never injected. Compare mastra-extended on main, which uses endpoint_var / model_var attachments per service.
  3. The chat model is renamed chatllm in compose.yaml (but stays chat in compose.local.yaml), so even default injection would yield LLM_URL, not the CHAT_URL the app expects.
  4. x-defang-llm: true is kept under models: here, while the samples already converted on main (mastra-extended, self-improving-mastra) omit it — worth deciding which is canonical.

The worker still depended on the removed chat/embedding provider
services, and no service was attached to the new top-level models, so
the CHAT_URL/CHAT_MODEL/EMBEDDING_URL/EMBEDDING_MODEL env vars read by
app/core/ai_clients.py were never injected. Attach both models to the
worker via endpoint_var/model_var (compose.local.yaml inherits this
through extends), rename the chat model back to match the local file,
and drop x-defang-llm to match the other samples on the new syntax.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sw7j9FYnbChZJbYArxbhuo
@defangdevs

Copy link
Copy Markdown
Contributor

Fixed the issues above in a follow-up commit on the parked branch (a0afb77c): the worker's stale depends_on entries are gone, both models are attached to the worker via endpoint_var/model_var (so CHAT_URL/CHAT_MODEL/EMBEDDING_URL/EMBEDDING_MODEL are injected again; compose.local.yaml inherits this through extends), the model is named chat in both files, and x-defang-llm is dropped to match the other converted samples.

Verified with defang compose config (built from defang main): both compose.yaml and compose.local.yaml render with the expected env vars on the worker.

Same command as before to update this PR (now brings both commits):

git fetch https://github.com/defangdevs/samples.git lio/new-model-runner
git push --force-with-lease origin FETCH_HEAD:refs/heads/lio/new-model-runner

@lionello
lionello force-pushed the lio/new-model-runner branch from b841033 to a0afb77 Compare July 21, 2026 12:20
@lionello
lionello had a problem deploying to deploy-changed-samples July 21, 2026 12:20 — with GitHub Actions Failure
@lionello
lionello had a problem deploying to deploy-changed-samples July 21, 2026 12:29 — with GitHub Actions Failure
@lionello
lionello marked this pull request as ready for review July 22, 2026 06:17
@lionello
lionello merged commit 2cd6e9b into main Jul 22, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Docker Model runner syntax in samples/CLI

2 participants