Enable embedding service in Docker Compose and update profiles - #6
Conversation
Source PR: appwrite#13429 Source head: 1835bf6
⛔ Shipwright · BlockedRecommendation: do not merge PR #6 · Tier
Findings (5)
Fireworks usage: 9,980 input · 804 output · 10,784 total tokens · $0.0027 · 14s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| COMPOSE_PROFILES="${COMPOSE_PROFILES},gitea" | ||
| fi | ||
|
|
||
| # The embedding container is resource-heavy and off by default, so start it and |
There was a problem hiding this comment.
Shipwright · CRITICAL
The CI workflow conditionally appends _APP_EMBEDDING=enabled to GITHUB_ENV only when matrix.service.embedding is true.
Impact: The CI workflow conditionally appends _APP_EMBEDDING=enabled to GITHUB_ENV only when matrix.service.embedding is true. However, the .env file now hardcodes _APP_EMBEDDING=disabled. If the CI setup sources .env after GITHUB_ENV, or if the compose environment resolution prefers .env over the shell environment, the embedding tests will run against a disabled API and fail. Conversely, if GITHUB_ENV wins, the container p…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| 'filter' => '' | ||
| ], | ||
| [ | ||
| 'name' => '_APP_EMBEDDING', |
There was a problem hiding this comment.
Shipwright · CRITICAL
The default value for _APP_EMBEDDING is 'enabled' in app/config/variables.php, but the .env file shipped with the repository sets _APP_EMBEDDING=disabled.
Impact: The default value for _APP_EMBEDDING is 'enabled' in app/config/variables.php, but the .env file shipped with the repository sets _APP_EMBEDDING=disabled. This creates a dangerous divergence: any deployment that does not explicitly copy the .env value (e.g., uses defaults, or an operator misses this line) will silently enable the embeddings API while the resource-heavy embedding container is NOT started (COMPOSE_PRO…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| // on the first write with the reason only in the logs. | ||
| // against an absent service with the reason only in the logs. Embeddings ran | ||
| // on every installation before it had a switch, so it stays on unless an | ||
| // operator turns it off; the resource-heavy container is what sits behind a |
There was a problem hiding this comment.
Shipwright · HIGH
The refactor of $products from string values to arrays with a default parameter ('documentsdb' => ['_APP_DOCUMENTSDB', 'disabled']) is non-obvious.
Impact: The refactor of $products from string values to arrays with a default parameter ('documentsdb' => ['_APP_DOCUMENTSDB', 'disabled']) is non-obvious. A reader must understand that the second array element is the fallback default for System::getEnv. The spread operator System::getEnv(...$products[$namespace]) obscures the argument order and makes it easy to swap the env var name and default value in future edits. Th…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| 'description' => 'Enables the embeddings API, backed by the resource-heavy appwrite-embedding container. That container sits behind the "embedding" Compose profile, so add "embedding" to COMPOSE_PROFILES to start it. Set this to "disabled" to have the /v1/embeddings routes return a service disabled error instead of reaching for the container. Default value is: enabled.', | ||
| 'introduction' => '2.0.0', | ||
| 'default' => 'enabled', | ||
| 'required' => false, |
There was a problem hiding this comment.
Shipwright · HIGH
The .env comment says 'The embeddings API is on by default and is switched off below until then' but the actual line sets _APP_EMBEDDING=disabled.
Impact: The .env comment says 'The embeddings API is on by default and is switched off below until then' but the actual line sets _APP_EMBEDDING=disabled. This contradicts the variables.php default of 'enabled'. An operator reading the .env comment may believe the API is off, while a fresh install using variables.php defaults will have it on. This documentation/behavior mismatch is a classic source of misconfiguration.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
What does this PR do?
Makes the resource-heavy
appwrite-embeddingcontainer off by default and gives self-hosters a clean way to run without it. On small servers the container can saturate RAM/CPU during function builds (reported on 1.9.6, Hetzner CPX21), and the only workaround was overriding the service with a dummy container.It mirrors the DocumentsDB/VectorsDB pattern from appwrite#13411: the feature is gated by an env flag, and the route returns a clean
service disablederror until an operator opts in — so a stock deployment never fails cryptically against an absent host.Changes:
docker-compose.ymlappwrite-embeddingmoves behind theembeddingCompose profile (not started unless the profile is active).- appwrite-embeddingfrom every service'sdepends_on(13 entries) — Compose auto-activates a profiled service's profile when a running service hard-depends on it, which would make it impossible to turn off. Services reach embedding lazily at runtime via_APP_EMBEDDING_ENDPOINT._APP_EMBEDDINGflag to the services that already carry_APP_VECTORSDB.app/config/variables.php— add_APP_EMBEDDING(defaultdisabled) so the installer emits it.app/controllers/shared/api.php— gate theembeddingsnamespace: returnGENERAL_SERVICE_DISABLED(to keys and privileged roles too) until_APP_EMBEDDING=enabled..env— default off:_APP_EMBEDDING=disabled,COMPOSE_PROFILES=mongodb..github/workflows/ci.yml— the Databases suite is the only one exercising/v1/embeddings/text, so it starts the container (embeddingprofile) and sets_APP_EMBEDDING=enabled.To enable embedding: add
embeddingtoCOMPOSE_PROFILES, set_APP_EMBEDDING=enabled, anddocker compose up -d.Test Plan
docker compose configverified:appwrite-embeddingabsent by default; present whenCOMPOSE_PROFILESincludesembedding. Full config validates (exit 0). No service still referencesappwrite-embeddingindepends_on.composer lintandcomposer analyzepass onapi.phpandvariables.php.testCreateTextEmbeddingsSuccessAndErrors,testDocumentNomicEmbeddingRoundTrip) run against a started embedding container with_APP_EMBEDDING=enabledvia the CI matrix flag._APP_EMBEDDING=disabled,/v1/embeddings/textreturnsgeneral_service_disabledinstead of failing against an absent container.Related PRs and Issues
appwrite-embeddingconsuming excess RAM/CPU during Python function builds.Checklist
Source merge-base:
50792641007b489259a9a005c4b932ed19ff12b9Source head:
1835bf6115caa597496800191cc9b7ec3a064b42