Conversation
edwinjosechittilappilly
left a comment
edwinjosechittilappilly
left a comment
Collaborator
There was a problem hiding this comment.
Good PR.
Few thought son tests and openserach component changes.
Connected Async
…nd exception Issues - #845 - #973 Summary Fix data ingestion failures caused by a race condition where the backend attempted to interact with Langflow and OpenSearch before those services were ready, and improve logging and vector index configuration. Bug Fix: Langflow and OpenSearch Readiness - Add new wait_for_langflow() utility with exponential backoff and jitter to poll Langflow's /health endpoint before proceeding with API key generation and client initialization (src/utils/langflow_utils.py) - Add LangflowNotReadyError exception raised when Langflow fails to become ready within the retry limit - Reorder AppClients.initialize() to create the langflow_http_client before calling wait_for_langflow() and get_langflow_api_key(), ensuring the HTTP client exists before health checks begin (src/config/settings.py) - Move Langflow API key generation (get_langflow_api_key()) to after the Langflow readiness check instead of the very start of initialization - Guard the onboarding endpoint with wait_for_langflow(), returning HTTP 503 if Langflow is unavailable (src/api/settings.py) - Extract init_index_when_ready() to explicitly sequence wait_for_opensearch() before init_index(), and call it from the onboarding endpoint (src/main.py) - Remove the redundant wait_for_opensearch() call inside init_index() so it can be reused without double-waiting - Add post-onboarding index initialization in startup_tasks() to recreate the OpenSearch index when the volume has been deleted but onboarding was already completed Vector Index Configuration - Increase ef_construction from 100 to 512 across all index creation paths for improved search recall (src/config/settings.py, src/utils/embedding_fields.py, src/utils/embeddings.py, flows/components/opensearch_multimodal.py, scripts/migrate_embedding_model_field.py) Logging Improvements - Add colored log output with ANSI codes for log level and timestamp, respecting NO_COLOR env var and non-TTY environments (src/utils/logging_config.py) - Document LOG_LEVEL and NO_COLOR environment variables in .env.example Code Quality - Fix trailing whitespace throughout src/api/settings.py (whitespace-only cleanup) Testing - Add comprehensive unit tests for wait_for_langflow() covering success, transient failures, retry exhaustion, backoff bounds, and edge cases (tests/unit/test_langflow_utils.py) - Add tests/unit/conftest.py with a no-op onboard_system fixture override so unit tests don't require running infrastructure
…nd exception Issues - #845 - #973 Summary Refactor wait_for_langflow to accept an explicit HTTP client and standardize test import paths from src.* to direct module references. Proposed Commit Message: Dependency Injection Refactor - Add `langflow_http_client` parameter to `wait_for_langflow()` so callers can pass the client directly rather than relying on a module-level import - Lazy-import `clients` from `config.settings` inside `wait_for_langflow` only when no client is provided, eliminating the circular import at module load - Update `AppClients.initialize()` to pass `self.langflow_http_client` explicitly Test Import Path Cleanup - Replace all `src.config.settings`, `src.main`, `src.api.*`, `src.services.*`, and `src.session_manager` imports with their direct (non-prefixed) counterparts in `conftest.py`, `test_api_endpoints.py`, `test_startup_ingest.py` - Prune redundant duplicate module keys from `sys.modules` cache-clearing blocks (e.g., both `src.api.router` and `api.router` were being cleared) Unit Test Isolation Improvement - Simplify `mock_langflow_client` fixture in `test_langflow_utils.py` — remove the `patch("utils.langflow_utils.clients")` monkey-patch; return a plain `AsyncMock` instead - Update all `wait_for_langflow()` unit test call sites to pass the mock client via the new `langflow_http_client` parameter for explicit, side-effect-free injection
…nd exception Issues - #845 - #973 Summary Lower the ef_construction value for OpenSearch KNN index configuration in both the backend settings and the multimodal Langflow component. This reduces memory and CPU overhead during index build time while maintaining acceptable recall for typical workloads.
…nd exception Issues - #845 - #973 Summary Expose backend operational environment variables — logging, timeouts, and concurrency — in both the Docker Compose configuration and Helm chart so they can be tuned without modifying source files. Docker Compose (openrag-backend): - Add LOG_LEVEL passthrough with default INFO - Add ACCESS_LOG passthrough with default true - Add NO_COLOR passthrough (empty default; isatty() already suppresses colors in containers) - Add LANGFLOW_TIMEOUT with default 2400s - Add LANGFLOW_CONNECT_TIMEOUT with default 30s - Add INGESTION_TIMEOUT with default 3600s - Add UPLOAD_BATCH_SIZE with default 25 - Add MAX_WORKERS with default 4 (documented upper cap) Helm chart (values.yaml + backend/deployment.yaml): Logging: - Add backend.logLevel (default: INFO), wired to LOG_LEVEL - Add backend.accessLog (default: true), wired to ACCESS_LOG - Add backend.noColor (default: false); injects NO_COLOR=1 only when true, avoiding the empty-string-in-environ footgun Timeouts: - Add backend.langflowTimeout (default: 2400), wired to LANGFLOW_TIMEOUT - Add backend.langflowConnectTimeout (default: 30), wired to LANGFLOW_CONNECT_TIMEOUT - Add backend.ingestionTimeout (default: 3600), wired to INGESTION_TIMEOUT Concurrency: - Add backend.uploadBatchSize (default: 25), wired to UPLOAD_BATCH_SIZE - Add backend.maxWorkers (default: empty = auto); only injected when non-empty to let the backend compute its own default Misc: - Align .env.example comment for INGESTION_TIMEOUT to match the OPTIONAL: prefix style of surrounding vars
edwinjosechittilappilly
changed the title
edwinjosechittilappilly
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses a race condition where the OpenRAG backend attempted to interact with Langflow and OpenSearch before those services were fully ready, causing data ingestion failures. It also adds colored log output, updates vector index configuration parameters, and includes comprehensive unit tests.
Changes:
- Adds
wait_for_langflow()utility with exponential backoff to ensure Langflow is healthy before API key generation and client initialization - Reorders
AppClients.initialize()to create the Langflow HTTP client before health checks - Guards the onboarding endpoint with Langflow readiness checks, returning HTTP 503 if unavailable
- Adds post-onboarding index initialization in
startup_tasks()to handle OpenSearch volume deletions - Adds colored log output with ANSI codes that respects
NO_COLORenvironment variable and TTY detection - Introduces
KNN_EF_CONSTRUCTIONandKNN_Mconstants for vector index configuration - Adds comprehensive unit tests for
wait_for_langflow()with proper mocking - Updates integration tests to use correct import paths (removing
src.prefix) - Documents new environment variables (
LOG_LEVEL,NO_COLOR) in.env.exampleand Helm values
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/langflow_utils.py | New module with Langflow health check retry logic using exponential backoff |
| src/config/settings.py | Reordered initialization to create HTTP client before health checks; introduced KNN constants |
| src/api/settings.py | Added Langflow readiness guard in onboarding endpoint; whitespace cleanup |
| src/main.py | Added init_index_when_ready() wrapper and post-onboarding index initialization |
| src/utils/logging_config.py | Implemented colored log output with NO_COLOR and TTY support |
| src/utils/embeddings.py | Updated to use KNN constants instead of hardcoded values |
| src/utils/embedding_fields.py | Updated to use KNN constants instead of hardcoded values |
| scripts/migrate_embedding_model_field.py | Updated to use KNN constants instead of hardcoded values |
| tests/unit/test_langflow_utils.py | Comprehensive unit tests for wait_for_langflow retry logic |
| tests/unit/conftest.py | No-op fixture override for unit tests to avoid infrastructure dependencies |
| tests/conftest.py | Updated imports to remove src. prefix |
| tests/integration/*.py | Updated imports to remove src. prefix |
| .env.example | Documented LOG_LEVEL and NO_COLOR environment variables |
| docker-compose.yml | Added environment variables for logging and timeouts |
| helm/openrag/values.yaml | Added logging, timeout, and concurrency configuration options |
| helm/openrag/templates/backend/deployment.yaml | Mapped new configuration values to environment variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues
Summary
Fix data ingestion failures caused by a race condition where the backend attempted to interact with Langflow and OpenSearch before those services were ready, and improve logging and vector index configuration.
Bug Fix: Langflow and OpenSearch Readiness
Vector Index Configuration
Logging Improvements
Code Quality
Testing
Screenshots