fix: Ensure Langflow .env variable definitions from LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT#1667
Merged
Merged
Conversation
…riables with default values when the comma separated list defined in LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT changes
Contributor
WalkthroughEnvVarManager adds default OpenSearch and Docling environment entries and a new EnsureRequiredEnvVars method that populates missing variables from LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT with "None"; the controller now calls this method when building the Langflow .env content. ChangesEnvironment variable defaults and required-vars enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
zzzming
approved these changes
May 22, 2026
rodageve
changed the title
rodageve
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
kubernetes/operator/internal/controller/openrag_controller.go (1)
504-513: ⚡ Quick winAdd a controller-level test for Docling precedence + required-var enforcement.
This method now combines two contracts (Docling URL priority and required-var backfill). A focused
buildLangflowEnvtest would prevent regressions thatEnvVarManagerunit tests won’t catch.Also applies to: 525-527
🤖 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 `@kubernetes/operator/internal/controller/openrag_controller.go` around lines 504 - 513, Add a controller-level unit test for buildLangflowEnv in openrag_controller.go that asserts (1) operator-managed DoclingComponents (o.Spec.DoclingComponents with Enabled and Serve) takes precedence over external o.Spec.Docling when both are present, and (2) required environment variables (e.g., DOCLING_SERVE_URL) are populated/backfilled correctly when using the operator-managed path and when using external Docling; implement test cases covering operator-managed only, external-only, and both-present scenarios and assert the resulting envVars map contains the expected URL and required keys so regressions in precedence or required-var enforcement are caught.kubernetes/operator/internal/controller/env_test.go (1)
352-361: ⚡ Quick winDerive required vars from
requiredVarsStrinstead of hard-coding.This block says it parses the list but currently hardcodes it. Splitting
requiredVarsStrkeeps the test aligned with future default-list changes.🤖 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 `@kubernetes/operator/internal/controller/env_test.go` around lines 352 - 361, Replace the hard-coded construction of requiredVars with a split of the existing requiredVarsStr so the test derives the list from the same source; specifically, where requiredVars is built (variable requiredVars in env_test.go), use strings.Split(requiredVarsStr, ",") (trimming spaces as needed) to populate requiredVars instead of the manual slice so future changes to requiredVarsStr are automatically reflected in the test.
🤖 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.
Inline comments:
In `@kubernetes/operator/internal/controller/env_test.go`:
- Around line 342-347: Test currently calls GetLangflowEnvVars(nil) which reads
the real process environment and makes the assertions flaky; instead construct
and pass a controlled defaults map (a copy of the manager's defaults or an
explicit map with the expected keys) or explicitly unset/sanitize
OPTLF_OPENSEARCH_* and OPTLF_DOCLING_SERVE_URL before calling GetLangflowEnvVars
so the function uses deterministic inputs; update the assertions that check
LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT and the similar block covering the
other assertions (the code around GetLangflowEnvVars,
LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT, OPTLF_OPENSEARCH_*, and
OPTLF_DOCLING_SERVE_URL) to use the sanitized map/copy to ensure the test is not
coupled to the host environment.
---
Nitpick comments:
In `@kubernetes/operator/internal/controller/env_test.go`:
- Around line 352-361: Replace the hard-coded construction of requiredVars with
a split of the existing requiredVarsStr so the test derives the list from the
same source; specifically, where requiredVars is built (variable requiredVars in
env_test.go), use strings.Split(requiredVarsStr, ",") (trimming spaces as
needed) to populate requiredVars instead of the manual slice so future changes
to requiredVarsStr are automatically reflected in the test.
In `@kubernetes/operator/internal/controller/openrag_controller.go`:
- Around line 504-513: Add a controller-level unit test for buildLangflowEnv in
openrag_controller.go that asserts (1) operator-managed DoclingComponents
(o.Spec.DoclingComponents with Enabled and Serve) takes precedence over external
o.Spec.Docling when both are present, and (2) required environment variables
(e.g., DOCLING_SERVE_URL) are populated/backfilled correctly when using the
operator-managed path and when using external Docling; implement test cases
covering operator-managed only, external-only, and both-present scenarios and
assert the resulting envVars map contains the expected URL and required keys so
regressions in precedence or required-var enforcement are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a10a5229-ee02-4dd5-87b7-416bedbf5d70
📒 Files selected for processing (3)
kubernetes/operator/internal/controller/env.gokubernetes/operator/internal/controller/env_test.gokubernetes/operator/internal/controller/openrag_controller.go
Comment on lines
+342
to
+347
| envVars := manager.GetLangflowEnvVars(nil) | ||
|
|
||
| // Verify LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT exists | ||
| requiredVarsStr, exists := envVars["LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT"] | ||
| assert.True(t, exists, "LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT should exist in defaults") | ||
| assert.NotEmpty(t, requiredVarsStr, "LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT should not be empty") |
Contributor
There was a problem hiding this comment.
Avoid host-environment coupling in this integration test.
GetLangflowEnvVars(nil) reads real process env, so these fixed "None" assertions can fail if OPTLF_OPENSEARCH_* or OPTLF_DOCLING_SERVE_URL is set in CI/runner env. Build from a copied defaults map (or explicitly sanitize those vars) to keep the test deterministic.
Also applies to: 372-375
🤖 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 `@kubernetes/operator/internal/controller/env_test.go` around lines 342 - 347,
Test currently calls GetLangflowEnvVars(nil) which reads the real process
environment and makes the assertions flaky; instead construct and pass a
controlled defaults map (a copy of the manager's defaults or an explicit map
with the expected keys) or explicitly unset/sanitize OPTLF_OPENSEARCH_* and
OPTLF_DOCLING_SERVE_URL before calling GetLangflowEnvVars so the function uses
deterministic inputs; update the assertions that check
LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT and the similar block covering the
other assertions (the code around GetLangflowEnvVars,
LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT, OPTLF_OPENSEARCH_*, and
OPTLF_DOCLING_SERVE_URL) to use the sanitized map/copy to ensure the test is not
coupled to the host environment.
zzzming
changed the title
github-actions
Bot
added
bug
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
kubernetes/operator/internal/controller/env_test.go (1)
353-358: ⚡ Quick winDerive required vars from
requiredVarsStrinstead of hardcoding the list.This integration test currently validates a static list rather than the actual default contract. If defaults change, this can drift and produce misleading results.
♻️ Suggested update
- requiredVars := []string{"JWT", "OPENRAG_QUERY_FILTER", "OPENSEARCH_PASSWORD", "OPENSEARCH_URL", - "OPENSEARCH_INDEX_NAME", "DOCLING_SERVE_URL", "DOCLING_TASK_ID", "OWNER", "OWNER_NAME", - "OWNER_EMAIL", "CONNECTOR_TYPE", "DOCUMENT_ID", "SOURCE_URL", "ALLOWED_USERS", - "ALLOWED_GROUPS", "FILENAME", "MIMETYPE", "FILESIZE", "SELECTED_EMBEDDING_MODEL", - "OPENAI_API_KEY", "ANTHROPIC_API_KEY", "WATSONX_API_KEY", "WATSONX_ENDPOINT", - "WATSONX_PROJECT_ID", "OLLAMA_BASE_URL"} + rawRequiredVars := strings.Split(requiredVarsStr, ",") + requiredVars := make([]string, 0, len(rawRequiredVars)) + for _, v := range rawRequiredVars { + v = strings.TrimSpace(v) + if v != "" { + requiredVars = append(requiredVars, v) + } + }🤖 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 `@kubernetes/operator/internal/controller/env_test.go` around lines 353 - 358, The test currently hardcodes requiredVars; change it to derive the expected list from the existing requiredVarsStr instead of the static slice. Replace the hardcoded requiredVars declaration with code that parses requiredVarsStr (split by the same delimiter used where requiredVarsStr is defined, trim spaces) and use that resulting slice for assertions so the test reflects the canonical default contract; update any variable names if needed to keep comparisons consistent with the rest of env_test.go.
🤖 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 `@kubernetes/operator/internal/controller/env_test.go`:
- Around line 353-358: The test currently hardcodes requiredVars; change it to
derive the expected list from the existing requiredVarsStr instead of the static
slice. Replace the hardcoded requiredVars declaration with code that parses
requiredVarsStr (split by the same delimiter used where requiredVarsStr is
defined, trim spaces) and use that resulting slice for assertions so the test
reflects the canonical default contract; update any variable names if needed to
keep comparisons consistent with the rest of env_test.go.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f759db2b-1946-4476-88e2-df4562537b2a
📒 Files selected for processing (1)
kubernetes/operator/internal/controller/env_test.go
ricofurtado
pushed a commit
that referenced
this pull request
May 23, 2026
…S_TO_GET_FROM_ENVIRONMENT (#1667) * Ensure we dynamically update the list of Langflow .env environment variables with default values when the comma separated list defined in LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT changes * fix tests * fix additional linting errors --------- Co-authored-by: rodageve <[email protected]>
ricofurtado
pushed a commit
that referenced
this pull request
May 23, 2026
…S_TO_GET_FROM_ENVIRONMENT (#1667) * Ensure we dynamically update the list of Langflow .env environment variables with default values when the comma separated list defined in LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT changes * fix tests * fix additional linting errors --------- Co-authored-by: rodageve <[email protected]>
ricofurtado
added a commit
that referenced
this pull request
May 26, 2026
* fix: Ensure SUCCESS status requires fetchable result in DoclingPollingService * style: ruff autofix (auto) * fix: Catch specific DoclingServeError when fetching task result after SUCCESS status * feat: update style for oss of the failed task in the task panel (#1647) * update style for oss of the failed task in the task panel * keep logic on click, remove unecessary useeffect * fix padding * wip implementing Saas style * utils to reshape error until backend provide info we need * utils to reshape error until backend provide info we need * utils to reshape error until backend provide info we need and fixinf fallbacks of isTotalFailure * utils to reshape error until backend provide into * have Saas style for failed and complete labelstatus and width and border * few style adjustment to follow codebase pattern * adjust succeed and partially succeed case * adding comment for TODO implementation or more clarity * remove carbon icon package and replace carbon icon * add incident-reporter-icon --------- Co-authored-by: Olfa Maslah <[email protected]> * fix: Encode IBM API key as Basic auth header (#1664) * Encode IBM API key as Basic auth header Add base64 encoding for the IBM auth path: import base64, construct a Basic auth token from X-Username and X-Api-Key (username:apikey), and store it in user.jwt_token and user.opensearch_credentials. Also set request.state.user before attaching the DB user ID so downstream code can access the created user object. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: restart deployment if env changes (#1665) * restart deployment if env changes * unit test * lint * fix: Ensure Langflow .env variable definitions from LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT (#1667) * Ensure we dynamically update the list of Langflow .env environment variables with default values when the comma separated list defined in LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT changes * fix tests * fix additional linting errors --------- Co-authored-by: rodageve <[email protected]> * chore: Retire openrag-mcp; switch docs to streamable HTTP (#1668) * Retire openrag-mcp; switch docs to streamable HTTP Remove the stdio-based MCP server and all in-repo MCP tooling, and update README to mark the package as retired. Deleted module files include the MCP entrypoint, server, config, registry and individual tools (chat, search, documents, settings). The README was rewritten to announce that openrag-mcp is retired, explain migration to the built-in streamable-HTTP /mcp endpoint, update Cursor/Claude examples to use URL+headers auth, list the new v1 API tools, and note that the last PyPI release is final. This change consolidates MCP functionality into the OpenRAG core and removes the subprocess/stdio implementation and its source code. * Mark MCP SDK retired and clean package metadata Update package metadata to reflect retirement and integration into the OpenRAG backend. Bump version to 0.3.0 and replace the project description with a retirement/migration note. Set Development Status to Inactive, remove explicit Python version classifiers, and clear runtime dependencies and the CLI script entrypoint. Also remove the hatch env pip-args setting; build-system and wheel package target remain unchanged. * chore: update uv.lock files after version bump * Update uv.lock --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: connector sync and override feature (#1663) * Add filename-based duplicate handling for connectors Add end-to-end support for filename-based duplicate handling on connector ingests. Frontend: send a new replace_duplicates flag with connector sync requests, perform a pre-sync duplicate check, and show a DuplicateHandlingDialog that lets users overwrite or skip duplicates when uploading from provider UI. Backend: propagate replace_duplicates through connector_router, request models, and connector services into the file processors. ConnectorFileProcessor and LangflowConnectorFileProcessor now check whether a filename already exists in the index and either fail the file task or delete the existing document before ingesting when replace_duplicates is true. Utilities/tests: clean_connector_filename now preserves original spacing/slashes and only enforces MIME-mapped extensions; get_filename_aliases adds underscore/sanitized variants so lookups match connector-indexed names. Add unit tests covering filename dedupe logic and filename alias behavior. * Use duplicateNames list and display names Replace numeric duplicateCount with a duplicateNames string[] across upload and dropdown flows so the UI can show the actual file names that would be overwritten. The duplicate-handling dialog now accepts duplicateNames, derives an effective count, and lists up to 5 duplicate filenames with an "… and N more" indicator; message labels and button text use the effective count. Toast messages and pending state in upload/[provider]/page.tsx and knowledge-dropdown.tsx were updated to pass and consume duplicateNames and to use duplicateNames.length for counts. * Update page.tsx * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: update OAuth prompt to consent for connector mutation (#1657) * fix: implement transient error handling for Docling result fetch * style: ruff autofix (auto) * refactor: remove unused import of Optional in docling_polling_service.py * refactor: change PollOutcome to use StrEnum for better type safety * refactor: enhance task status endpoints with structured failure metadata * style: ruff autofix (auto) * revert "style: ruff autofix (auto)" This reverts commit bc8be33. * style: ruff autofix (auto) * fix: Ensure SUCCESS status requires fetchable result in DoclingPollingService * style: ruff autofix (auto) * fix: Catch specific DoclingServeError when fetching task result after SUCCESS status * fix: implement transient error handling for Docling result fetch * style: ruff autofix (auto) * refactor: remove unused import of Optional in docling_polling_service.py * refactor: change PollOutcome to use StrEnum for better type safety * refactor: enhance task status endpoints with structured failure metadata * style: ruff autofix (auto) * revert "style: ruff autofix (auto)" This reverts commit bc8be33. * style: ruff autofix (auto) * Update tests/unit/test_task_service_get_task_status2.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * style: ruff autofix (auto) * fix: handle timeout during Docling result fetch after SUCCESS status * fix: update task status checks to use enum values for consistency * fix: enhance failure metadata for duplicate file errors in ingestion * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Wallgau <[email protected]> Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: ming <[email protected]> Co-authored-by: rodageve <[email protected]> Co-authored-by: rodageve <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This was referenced Jun 4, 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
2 participants
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.
Description:
✅ Dynamic reconciliation - Works with any value of LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT (from defaults, operator env with OPTLF_ prefix, or CR spec)
✅ Backward compatible - Doesn't change existing behavior for variables that already have values
✅ Prevents missing variable errors - Langflow components will always find the variables they expect
✅ Consistent DOCLING_SERVE_URL - Both backend and Langflow use the same URL with proper priority:
Operator-managed Docling (DoclingComponents) > External Docling > Default "None"
✅ Handles edge cases - Empty strings, whitespace, missing list, custom lists all work correctly
✅ Three-level priority maintained - CR spec > Operator env > Defaults (unchanged)
Files Modified
kubernetes/operator/internal/controller/env.go - Added defaults and helper function
kubernetes/operator/internal/controller/openrag_controller.go - Integrated helper and fixed DOCLING_SERVE_URL logic
kubernetes/operator/internal/controller/env_test.go - Added comprehensive unit tests
Summary by CodeRabbit
New Features
Tests