feat: deployment-profile based pre-flight checks - #14393
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesProduction deployment preflight
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant SettingsService
participant Preflight
participant StorageService
participant Database
participant Lifespan
CLI->>SettingsService: resolve deployment profile
CLI->>Preflight: run production preflight
Preflight->>StorageService: check storage readiness
Preflight->>Database: test database and pgVector
Preflight-->>Lifespan: allow startup or raise abort
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (8)
src/backend/tests/unit/test_cli_preflight.py (2)
243-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
osat module level instead of__import__("os").Line 273 already uses a local
import osin another test. Use a single module-level import for both.♻️ Proposed change
- assert PREFLIGHT_COMPLETED_ENV not in __import__("os").environ + assert PREFLIGHT_COMPLETED_ENV not in os.environ🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/tests/unit/test_cli_preflight.py` around lines 243 - 254, Update the test module imports to add a module-level os import, then replace the __import__("os").environ usage in test_ensure_is_noop_in_dev and the local import in the other test with that shared os symbol.
127-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the two
check_readinessimplementations.These tests replace
StorageServiceFactory.createwith a fake, so they verify only the reason-to-remediation mapping inprobe_storage. The new probe implementations stay untested: the default filesystem probe inStorageService.check_readiness(writable path and theOSErrortounwritablepath) and the S3 status-code classification inS3StorageService.check_readiness(bucket-missing,access-denied,unreachable). Cover the filesystem probe with a realtmp_pathdirectory and a read-only directory, and cover the S3 branches with a stub client that raises errors carrying aresponsedict.As per coding guidelines: "For new backend implementations or bug fixes, ensure corresponding pytest test files are included ... and verify the tests actually cover the new or changed behavior rather than acting as placeholders."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/tests/unit/test_cli_preflight.py` around lines 127 - 156, Add direct pytest coverage for StorageService.check_readiness using a real tmp_path directory, including writable success and an OSError case that maps to unwritable. Add S3StorageService.check_readiness tests with a stub client whose errors expose response dictionaries, covering bucket-missing, access-denied, and unreachable classifications; keep these separate from probe_storage factory-mocking tests so the implementations themselves are exercised.Source: Coding guidelines
src/backend/base/langflow/services/storage/s3.py (1)
191-193: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding the
head_bucketprobe with an explicit timeout.
_get_client()uses default botocore timeouts and retries. If the endpoint blackholes packets, this preflight probe can block boot for a long time before it reportsunreachable. Pass a shortbotocore.config.Config(connect/read timeout plus a low retry count) for the readiness client, or wrap the call inasyncio.timeout.♻️ Example: dedicated short-timeout client for the probe
+ from botocore.config import Config + try: - async with self._get_client() as s3_client: + probe_config = Config(connect_timeout=5, read_timeout=5, retries={"max_attempts": 1}) + async with self.session.create_client("s3", config=probe_config) as s3_client: await s3_client.head_bucket(Bucket=self.bucket_name)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/base/langflow/services/storage/s3.py` around lines 191 - 193, Update the readiness probe around S3Storage’s head_bucket call to enforce a short explicit timeout, preferably by using a dedicated botocore Config with bounded connect/read timeouts and low retries when creating the probe client, or by wrapping the operation in asyncio.timeout. Keep the existing unreachable handling while ensuring blackholed endpoints cannot block startup indefinitely.src/backend/base/langflow/main.py (1)
620-647: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the two identical abort handlers.
The
UnsupportedPostgreSQLVersionErrorandPreflightAbortErrorhandlers have the same body: flush both streams, signal the parent, thenos._exit(3). Combine them into oneexceptclause so the exit contract stays in one place.♻️ Proposed change
- except UnsupportedPostgreSQLVersionError: - # Normally caught by the pre-flight check in __main__.py - # before the server starts. If we get here anyway (e.g. - # direct uvicorn invocation via ``make backend``), exit - # immediately and tell the parent (reloader) to stop. - import signal - - sys.stdout.flush() - sys.stderr.flush() - with suppress(ProcessLookupError, PermissionError): - os.kill(os.getppid(), signal.SIGTERM) - os._exit(3) - except PreflightAbortError: - # Same rationale as UnsupportedPostgreSQLVersionError above: on the - # `langflow run` route this is caught before the server starts. If we - # reach here, boot came via a CLI-bypassing entrypoint (make backend, - # uvicorn --factory) with deployment_profile=prod and a required - # dependency missing. The summary has already been printed — exit - # immediately and tell the parent (reloader) to stop. - import signal - + except (UnsupportedPostgreSQLVersionError, PreflightAbortError): + # Normally caught before the server starts: the Postgres version check + # and the production preflight both run in __main__.py. Reaching here + # means boot came via a CLI-bypassing entrypoint (make backend, + # uvicorn --factory). The diagnostic output is already printed, so exit + # immediately and tell the parent (reloader) to stop. + import signal + sys.stdout.flush()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/base/langflow/main.py` around lines 620 - 647, Merge the UnsupportedPostgreSQLVersionError and PreflightAbortError handlers in the lifespan exception flow into a single multi-exception except clause. Preserve the existing shared behavior—flush stdout and stderr, signal the parent process while suppressing lookup/permission errors, then exit with status 3—and retain any necessary explanatory comments.src/backend/base/langflow/cli/preflight.py (2)
336-361: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTear down the throwaway queue service like the cache probe does.
probe_cacheandprobe_storageboth callteardown()in afinallyblock to keep no socket in the pre-fork parent.probe_shared_queuedropsqueue_servicewithout teardown on every path, including the non-Redis branch and the failure branch ofis_connected(). The inline comment states the guarantee for one method on one class only. Add the same symmetric cleanup so the invariant does not depend on the internals ofis_connected().♻️ Proposed change
queue_service = JobQueueServiceFactory().create(settings_service) - if isinstance(queue_service, RedisJobQueueService): - if await queue_service.is_connected(): - return CheckResult("ok", f"redis reachable ({queue_service.connection_target})") - return CheckResult( - "warn", - f"redis queue configured but unreachable at {queue_service.connection_target}", - "Start Redis or fix LANGFLOW_REDIS_QUEUE_* settings; jobs will not be distributed.", - ) - return CheckResult("ok", f"configured ({queue_type})") + try: + if isinstance(queue_service, RedisJobQueueService): + if await queue_service.is_connected(): + return CheckResult("ok", f"redis reachable ({queue_service.connection_target})") + return CheckResult( + "warn", + f"redis queue configured but unreachable at {queue_service.connection_target}", + "Start Redis or fix LANGFLOW_REDIS_QUEUE_* settings; jobs will not be distributed.", + ) + return CheckResult("ok", f"configured ({queue_type})") + finally: + teardown = getattr(queue_service, "teardown", None) + if teardown is not None: + with contextlib.suppress(Exception): + await teardown()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/base/langflow/cli/preflight.py` around lines 336 - 361, Update probe_shared_queue to always tear down the created queue_service in a finally block, covering non-Redis, successful connectivity, and failed is_connected() paths. Keep the existing CheckResult behavior unchanged while ensuring cleanup is performed symmetrically with probe_cache and probe_storage.
368-408: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
ALL_CHECKSis a snapshot, socollect_outcomescan ignore later list changes.
ALL_CHECKSis built once at import time._execute_and_renderreadsREQUIRED_CHECKSandDEGRADED_CHECKSat call time, so the two entry points can disagree after either list is reassigned (the tests reassignpreflight.REQUIRED_CHECKS). Compute the default insidecollect_outcomesto keep one behavior.♻️ Proposed change
- selected = checks if checks is not None else ALL_CHECKS + selected = checks if checks is not None else [*REQUIRED_CHECKS, *DEGRADED_CHECKS]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/base/langflow/cli/preflight.py` around lines 368 - 408, Update collect_outcomes so its default selection is built at call time from the current REQUIRED_CHECKS and DEGRADED_CHECKS lists instead of using the import-time ALL_CHECKS snapshot. Preserve the explicit checks argument behavior, and align the default path with _execute_and_render when either list is reassigned.src/backend/base/langflow/__main__.py (1)
405-411: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider constraining the option values at the CLI layer.
The option accepts any string. An invalid value such as
--deployment-profile stagingreachessettings_service.set(...)in the CLI-arg loop and raises a Pydantic validation error insideprogress.step(1), so the user sees a traceback instead of a usage error. Addclick.Choice(["dev", "prod"])so Typer rejects the value with a clean message.♻️ Proposed change
deployment_profile: str | None = typer.Option( # noqa: ARG001 — applied to settings via the CLI-arg loop below None, + click_type=click.Choice(["dev", "prod"], case_sensitive=False), help="Deployment profile: 'dev' (default) or 'prod'. 'prod' runs fail-loud "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/base/langflow/__main__.py` around lines 405 - 411, Constrain the deployment_profile Typer option to the supported values by using click.Choice(["dev", "prod"]) instead of an unrestricted string type. Keep the existing default, help text, and CLI-argument loop behavior unchanged so invalid values are rejected as usage errors before reaching settings_service.set.src/lfx/src/lfx/services/storage/service.py (1)
18-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated storage readiness contract in
lfxandlangflow-base. Both packages define the sameStorageReadinessdataclass and the same defaultcheck_readinessbody, including the same reason vocabulary. The two definitions can drift, and a value produced by one package fails anisinstancecheck against the other.langflow.cli.preflightalready imports fromlfx, so one source of truth is available.
src/lfx/src/lfx/services/storage/service.py#L18-L36: keep this definition as the single source of truth and keep the default probe at lines 212-239 here.src/backend/base/langflow/services/storage/service.py#L21-L39: import and re-exportStorageReadinessfromlfx.services.storage.serviceinstead of redefining it, and delegate the defaultcheck_readinessbody at lines 51-74 to thelfximplementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lfx/src/lfx/services/storage/service.py` around lines 18 - 36, Use src/lfx/src/lfx/services/storage/service.py lines 18-36 as the single StorageReadiness definition and retain its default check_readiness implementation at lines 212-239. In src/backend/base/langflow/services/storage/service.py lines 21-39, remove the duplicate dataclass and import/re-export StorageReadiness from lfx.services.storage.service; update the default check_readiness at lines 51-74 to delegate to the lfx implementation while preserving the existing API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/backend/base/langflow/__main__.py`:
- Around line 405-411: Constrain the deployment_profile Typer option to the
supported values by using click.Choice(["dev", "prod"]) instead of an
unrestricted string type. Keep the existing default, help text, and CLI-argument
loop behavior unchanged so invalid values are rejected as usage errors before
reaching settings_service.set.
In `@src/backend/base/langflow/cli/preflight.py`:
- Around line 336-361: Update probe_shared_queue to always tear down the created
queue_service in a finally block, covering non-Redis, successful connectivity,
and failed is_connected() paths. Keep the existing CheckResult behavior
unchanged while ensuring cleanup is performed symmetrically with probe_cache and
probe_storage.
- Around line 368-408: Update collect_outcomes so its default selection is built
at call time from the current REQUIRED_CHECKS and DEGRADED_CHECKS lists instead
of using the import-time ALL_CHECKS snapshot. Preserve the explicit checks
argument behavior, and align the default path with _execute_and_render when
either list is reassigned.
In `@src/backend/base/langflow/main.py`:
- Around line 620-647: Merge the UnsupportedPostgreSQLVersionError and
PreflightAbortError handlers in the lifespan exception flow into a single
multi-exception except clause. Preserve the existing shared behavior—flush
stdout and stderr, signal the parent process while suppressing lookup/permission
errors, then exit with status 3—and retain any necessary explanatory comments.
In `@src/backend/base/langflow/services/storage/s3.py`:
- Around line 191-193: Update the readiness probe around S3Storage’s head_bucket
call to enforce a short explicit timeout, preferably by using a dedicated
botocore Config with bounded connect/read timeouts and low retries when creating
the probe client, or by wrapping the operation in asyncio.timeout. Keep the
existing unreachable handling while ensuring blackholed endpoints cannot block
startup indefinitely.
In `@src/backend/tests/unit/test_cli_preflight.py`:
- Around line 243-254: Update the test module imports to add a module-level os
import, then replace the __import__("os").environ usage in
test_ensure_is_noop_in_dev and the local import in the other test with that
shared os symbol.
- Around line 127-156: Add direct pytest coverage for
StorageService.check_readiness using a real tmp_path directory, including
writable success and an OSError case that maps to unwritable. Add
S3StorageService.check_readiness tests with a stub client whose errors expose
response dictionaries, covering bucket-missing, access-denied, and unreachable
classifications; keep these separate from probe_storage factory-mocking tests so
the implementations themselves are exercised.
In `@src/lfx/src/lfx/services/storage/service.py`:
- Around line 18-36: Use src/lfx/src/lfx/services/storage/service.py lines 18-36
as the single StorageReadiness definition and retain its default check_readiness
implementation at lines 212-239. In
src/backend/base/langflow/services/storage/service.py lines 21-39, remove the
duplicate dataclass and import/re-export StorageReadiness from
lfx.services.storage.service; update the default check_readiness at lines 51-74
to delegate to the lfx implementation while preserving the existing API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cc268c1c-5a23-479d-964d-ed4cf05d8198
📒 Files selected for processing (9)
src/backend/base/langflow/__main__.pysrc/backend/base/langflow/cli/preflight.pysrc/backend/base/langflow/cli/progress.pysrc/backend/base/langflow/main.pysrc/backend/base/langflow/services/storage/s3.pysrc/backend/base/langflow/services/storage/service.pysrc/backend/tests/unit/test_cli_preflight.pysrc/lfx/src/lfx/services/settings/groups/server.pysrc/lfx/src/lfx/services/storage/service.py
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
55dc77d to
f6ec8d0
Compare
github-actions
Bot
added
enhancement
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14393 +/- ##
==================================================
+ Coverage 61.63% 62.93% +1.30%
==================================================
Files 2416 2379 -37
Lines 242162 242289 +127
Branches 36186 33939 -2247
==================================================
+ Hits 149249 152485 +3236
+ Misses 90995 87883 -3112
- Partials 1918 1921 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
added
enhancement
f6ec8d0 to
413264f
Compare
github-actions
Bot
added
enhancement
…ify with preflight #14393) instead of LANGFLOW_PROD
…ify with preflight #14393) instead of LANGFLOW_PROD
Added conditional check, if richer service is selected like redis but is not reachable, bootup fails.
bceb04c to
12abc7e
Compare
github-actions
Bot
added
enhancement
1 participant
Production profile pre-flight checks for infra services
Adds a
--deployment-profile(dev|prod) flag that, when set toprod, runs a set of fail-loud infrastructure checks before any worker is spawned. A misconfigured production deployment now aborts cleanly with actionable remediation instead of coming up half-working.Motivation
Today Langflow will happily boot in production with local filesystem storage, an auto-generated per-boot secret key, or a missing pgvector database — failures that only surface later as broken file persistence, invalidated JWTs across replicas, or runtime errors. This adds an explicit, opt-in production gate that surfaces every misconfiguration up front, in a single pass.
What's included
deployment_profilesetting (src/lfx/.../settings/groups/server.py) —dev(default) orprod, settable viaLANGFLOW_DEPLOYMENT_PROFILEor--deployment-profile. Normalizing validator acceptsPROD,prod, etc.langflow/cli/preflight.py) running two tiers of checks:vectorextension installed).StorageReadinessdataclass +check_readiness()onStorageService(local writability sentinel) andS3StorageService(credential resolution +head_bucket, distinguishing missing-creds / bucket-missing / access-denied / unreachable). Mirrored in bothlangflowandlfx.runexecutes preflight in the parent process before Gunicorn forks workers, so a bad config exits non-zero and no worker starts.make backend,uvicorn --factory, raw Gunicorn), before services/migrations initialize.LANGFLOW_PREFLIGHT_COMPLETEDenv sentinel (inherited across fork) ensures the checks run exactly once per process tree.Design notes
dev(the default) is a complete no-op.Tests
src/backend/tests/unit/test_cli_preflight.py(296 lines) covering profile normalization, per-check pass/fail/degraded outcomes, run-once sentinel behavior, and abort semantics.Files
cli/preflight.pycli/progress.py__main__.py--deployment-profileflag + pre-fork invocationmain.pyPreflightAbortErrorhandlingstorage/service.py,storage/s3.pyStorageReadiness+check_readiness()lfx/.../settings/groups/server.pydeployment_profilesetting + validatorlfx/.../storage/service.pyStorageReadinesstests/unit/test_cli_preflight.pySummary by CodeRabbit
New Features
Bug Fixes
Tests
Testing:
1. Fastest smoke test (no infra needed)
Dev = nothing happens:
Expect: normal boot, no "production preflight" section.
Prod = fails loud and aborts (default sqlite DB passes; storage/secret/pgvector fail):
Expect: a "Deployment profile: prod" section, red ✗ for File storage / Encryption secret key /
Vector backend, a "Preflight failed — N checks did not pass. Aborting boot." summary, and the
process exits without starting the server.
2. Exercise each check in isolation (fast, no full boot)
This runs just the probes and prints their status:
Prefix env vars to flip individual checks (examples below).
LANGFLOW_DATABASE_URLLANGFLOW_DATABASE_URL=postgresql://u:p@127.0.0.1:1/xLANGFLOW_STORAGE_TYPE=s3+ real bucket/creds (see §5)LANGFLOW_STORAGE_TYPE=local(mandated external in prod)LANGFLOW_SECRET_KEY=$(python3 -c "print('x'*40)")vectorextension (see §5)PGVECTOR_CONNECTION_STRINGLANGFLOW_DO_NOT_TRACK=trueLANGFLOW_CACHE_TYPE=redis+ Redis upLANGFLOW_CACHE_TYPE=redis+ Redis downLANGFLOW_JOB_QUEUE_TYPE=redis+ Redis upLANGFLOW_JOB_QUEUE_TYPE=redis+ Redis downCache/queue behavior (the key new rule)
Expect in the second case: red ✗ under Degraded services for Cache and Shared queue, and the
remediation tells you to fix the backend or unset the flag to boot degraded.
3. Route coverage — the check must fire regardless of how the app starts
a)
langflow run(CLI path): covered by §1.b)
make backend/ uvicorn--factory(bypasses the CLI): the lifespan enforces it andhard-exits the worker.
Expect: preflight renders, then the worker exits (code 3) instead of serving. (Confirms the
check isn't CLI-only.)
4. Run-once verification (multi-worker shouldn't probe N times)
Expect: the "production preflight" section prints once, not once per worker. (The parent runs
it pre-fork and sets
LANGFLOW_PREFLIGHT_COMPLETED=1, which workers inherit and skip.)5. Full GREEN path (QA — needs infra)
Spin up Postgres+pgvector and Redis with Docker:
Then boot with everything wired (storage still needs real S3 — or MinIO, see note):
Expect: all ✓/⚠, "Preflight passed … Continuing boot", server starts.
Notes:
requires
AWS_ENDPOINT_URL=http://localhost:9000and a pre-created bucket.PGVECTOR_CONNECTION_STRINGat a Postgres where you have not run
CREATE EXTENSION vector;→ red ✗ "reachable, but the'vector' extension is not installed".
Cleanup:
docker rm -f lf-pg lf-redis