Skip to content

feat: shards and replicas#1735

Open
phact wants to merge 2 commits into
mainfrom
shards-and-replicas
Open

feat: shards and replicas#1735
phact wants to merge 2 commits into
mainfrom
shards-and-replicas

Conversation

@phact

@phact phact commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Added configurable OpenSearch shard and replica count settings for newly created indices, enabling optimization of index performance and resilience through environment variables.
  • Documentation

    • Updated configuration reference with new OpenSearch index tuning parameters.
  • Tests

    • Added test coverage for OpenSearch index configuration settings.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds configurable OpenSearch index sharding and replica settings across all deployment layers. Environment variables OPENRAG_OPENSEARCH_NUMBER_OF_SHARDS and OPENRAG_OPENSEARCH_NUMBER_OF_REPLICAS are introduced, with defaults of 1 shard and 0 replicas, propagated through Kubernetes CRD, Helm charts, docker-compose, and TUI configuration, then enforced during index creation and runtime replica synchronization.

Changes

OpenSearch Shard/Replica Configuration

Layer / File(s) Summary
Configuration Schema and Constants
kubernetes/operator/api/v1alpha1/openrag_types.go, kubernetes/operator/config/crd/bases/openr.ag_openrags.yaml, kubernetes/helm/openrag/values.yaml, src/config/settings.py, flows/components/opensearch_multimodal.py
Kubernetes CRD schema adds NumberOfShards and NumberOfReplicas fields with kubebuilder validation; Helm chart sets defaults; Python modules define configuration constants with environment variable clamping to enforce minimums.
Environment and Deployment Configuration
.env.example, docker-compose.yml, kubernetes/helm/openrag/templates/backend/backend-dotenv.yaml, kubernetes/helm/openrag/templates/langflow/langflow-dotenv.yaml, kubernetes/operator/internal/controller/env.go
.env and docker-compose updated with new shard/replica environment variables; Helm templates map values to container environment; operator controller provides environment defaults for backend and langflow services.
Kubernetes Operator Controller Integration
kubernetes/operator/internal/controller/openrag_controller.go, kubernetes/operator/internal/controller/openrag_controller_test.go
Controller propagates CRD OpenSearch spec fields into backend and langflow .env generation; test verifies shard/replica values are correctly included.
Backend Index Body Creation
src/utils/embeddings.py, flows/components/opensearch_multimodal.py
Index creation functions import configuration constants and apply configured shard/replica counts to index body settings instead of hardcoded values.
Index Initialization and Replica Enforcement
src/utils/opensearch_init.py
New _ensure_index_replicas() helper retrieves current replica settings and updates to match configuration when mismatched; documents, knowledge-filters, API keys, and DLS principal indices are created with or synced to configured settings.
TUI Configuration Management
src/tui/config_fields.py, src/tui/managers/env_manager.py
Configuration fields added for opensearch_number_of_shards and opensearch_number_of_replicas with defaults; environment manager reads and writes these variables to .env file; type annotations modernized from Optional[...] to `
Documentation and Tests
docs/docs/reference/configuration.mdx, kubernetes/operator/README.md, kubernetes/operator/config/samples/*, tests/unit/test_embedding_fields.py
Configuration reference documents new environment variables; Kubernetes examples show shard/replica options; unit test verifies constants are applied to generated index bodies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement

Suggested reviewers

  • mfortman11
  • mendonk
  • zzzming
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.95% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: shards and replicas' directly describes the main feature added across all files: making OpenSearch shard and replica counts configurable instead of hardcoded.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch shards-and-replicas

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Trivy (0.69.3)

Trivy execution failed: 2026-06-02T18:02:04Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: kubernetes scan error: fs filter error: fs filter error: walk error range error: stat .coderabbit-opengrep-fallback.yml: no such file or directory: range error: stat .coderabbit-opengrep-fallback.yml: no such file or directory


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added documentation 📘 Improvements or additions to documentation backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) docker tests labels Jun 2, 2026
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Build successful! ✅
Deploying docs draft.
Deploy successful! View draft

@github-actions github-actions Bot added enhancement 🔵 New feature or request and removed documentation 📘 Improvements or additions to documentation labels Jun 2, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
kubernetes/operator/internal/controller/env.go (1)

34-34: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Expose the new OpenSearch settings through Langflow's env allowlist.

Adding defaults here is not enough: Line 34 still omits both new names from LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT, so Langflow components/flows will not see the shard/replica values even though they exist in the pod env.

Suggested fix
-			"LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT": "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,ALLOWED_PRINCIPALS,FILENAME,MIMETYPE,FILESIZE,SELECTED_EMBEDDING_MODEL,OPENAI_API_KEY,ANTHROPIC_API_KEY,WATSONX_API_KEY,WATSONX_ENDPOINT,WATSONX_PROJECT_ID,OLLAMA_BASE_URL,OPENRAG_INGEST_URL,OPENRAG_INGEST_TOKEN,OPENRAG_INGEST_RUN_ID,OPENRAG_INGEST_BATCH_SIZE",
+			"LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT": "JWT,OPENRAG_QUERY_FILTER,OPENSEARCH_PASSWORD,OPENSEARCH_URL,OPENSEARCH_INDEX_NAME,OPENRAG_OPENSEARCH_NUMBER_OF_SHARDS,OPENRAG_OPENSEARCH_NUMBER_OF_REPLICAS,DOCLING_SERVE_URL,DOCLING_TASK_ID,OWNER,OWNER_NAME,OWNER_EMAIL,CONNECTOR_TYPE,DOCUMENT_ID,SOURCE_URL,ALLOWED_USERS,ALLOWED_GROUPS,ALLOWED_PRINCIPALS,FILENAME,MIMETYPE,FILESIZE,SELECTED_EMBEDDING_MODEL,OPENAI_API_KEY,ANTHROPIC_API_KEY,WATSONX_API_KEY,WATSONX_ENDPOINT,WATSONX_PROJECT_ID,OLLAMA_BASE_URL,OPENRAG_INGEST_URL,OPENRAG_INGEST_TOKEN,OPENRAG_INGEST_RUN_ID,OPENRAG_INGEST_BATCH_SIZE",

Also applies to: 77-82

🤖 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.go` at line 34, Update the
LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT allowlist to include the new
OpenSearch shard/replica env names so Langflow can read them at runtime;
specifically add OPENSEARCH_SHARDS and OPENSEARCH_REPLICAS to the
comma-separated string value of LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT (the
same constant appears again further down and must be updated there as well) so
components/flows can access these variables.
src/tui/managers/env_manager.py (1)

715-780: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Full setup still doesn't surface the new shard/replica knobs.

EnvConfig, _env_attr_map(), and save_env_file() all support these values now, but get_full_setup_fields() never exposes them. The guided setup path will keep users pinned to the hard-coded 1/0 defaults, so this feature is only partially configurable.

Suggested fix
         optional_fields = [
             (
                 "disable_ingest_with_langflow",
                 "Disable Langflow Ingestion (optional)",
                 "False",
                 False,
             ),
             (
                 "ingest_sample_data",
                 "Ingest Sample Data (optional)",
                 "True",
                 False,
             ),
             (
                 "opensearch_index_name",
                 "OpenSearch Index Name",
                 "documents",
                 False,
             ),
+            (
+                "opensearch_number_of_shards",
+                "OpenSearch Primary Shards",
+                "1",
+                False,
+            ),
+            (
+                "opensearch_number_of_replicas",
+                "OpenSearch Replica Shards",
+                "0",
+                False,
+            ),
             (
                 "webhook_base_url",
                 "Webhook Base URL (optional)",
                 "https://your-domain.com",
                 False,
🤖 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/tui/managers/env_manager.py` around lines 715 - 780,
get_full_setup_fields() never includes the new OpenSearch shard/replica
environment keys, so guided setup keeps hard-coded 1/0 defaults; add the two env
keys (the exact names implemented in EnvConfig/_env_attr_map/save_env_file) into
the optional_fields list inside get_full_setup_fields with user-friendly labels
like "OpenSearch Index Shards" and "OpenSearch Index Replicas" and sensible
defaults ("1" and "0") and booleans matching other optional items so the guided
setup surfaces and saves those values.
🧹 Nitpick comments (1)
src/utils/embeddings.py (1)

63-70: 💤 Low value

Settings shape is inconsistent with the rest of the codebase.

Here number_of_shards/number_of_replicas are placed flat under settings, as siblings of index: {knn: True}, whereas the knowledge_filters body in opensearch_init.py (Lines 281-284) nests both under settings.index. OpenSearch normalizes the flat form to index.*, so this works, but the divergence is a maintenance smell — consider nesting them under index for consistency.

♻️ Optional: nest under index
     return {
         "settings": {
-            "index": {"knn": True},
-            "number_of_shards": OPENSEARCH_NUMBER_OF_SHARDS,
-            "number_of_replicas": OPENSEARCH_NUMBER_OF_REPLICAS,
+            "index": {
+                "knn": True,
+                "number_of_shards": OPENSEARCH_NUMBER_OF_SHARDS,
+                "number_of_replicas": OPENSEARCH_NUMBER_OF_REPLICAS,
+            },
         },
         "mappings": {"properties": properties},
     }

Note: if you adopt this, the unit test assertions in tests/unit/test_embedding_fields.py (Lines 139-140) that read body["settings"]["number_of_shards"] must be updated to body["settings"]["index"][...].

🤖 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/utils/embeddings.py` around lines 63 - 70, The settings dict returned by
the function (the block returning {"settings": {...}, "mappings": {"properties":
properties}}) places number_of_shards and number_of_replicas at the top of
settings, which is inconsistent with other code that nests them under
settings.index; update the returned structure so that "settings.index" contains
{"knn": True, "number_of_shards": OPENSEARCH_NUMBER_OF_SHARDS,
"number_of_replicas": OPENSEARCH_NUMBER_OF_REPLICAS} (keep "mappings":
{"properties": properties} unchanged) and adjust any unit tests that assert
body["settings"]["number_of_shards"] to check
body["settings"]["index"]["number_of_shards"] instead.
🤖 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 `@flows/components/opensearch_multimodal.py`:
- Line 5: Remove the unused "import os" from
flows/components/opensearch_multimodal.py and instead import any needed
configuration values from config/settings.py (e.g., from config.settings import
SOME_SETTING) so no os.environ reads remain in this module; update any local
references that would have used os.environ to use the corresponding settings
symbols and ensure imports and names (e.g., any function or class in this file
that relied on env vars) are updated accordingly.
- Around line 34-43: Remove the local _get_min_env_int helper and the
OPENSEARCH_NUMBER_OF_SHARDS / OPENSEARCH_NUMBER_OF_REPLICAS constant definitions
and instead import the existing constants from config.settings; update the
imports near the other config imports (around line where other config values are
imported) to pull OPENSEARCH_NUMBER_OF_SHARDS and OPENSEARCH_NUMBER_OF_REPLICAS
from config.settings, then delete the _get_min_env_int function and the two
local constant assignments in flows/components/opensearch_multimodal.py so all
config values come from config/settings.py as required.

In `@src/tui/config_fields.py`:
- Line 10: Remove the unused import Optional from the import statement (current
line imports "Optional") in config_fields.py; update the top-level typing import
so it no longer imports Optional (leaving only used names) to satisfy Ruff and
eliminate the unused-import error.
- Around line 125-142: The two ConfigField instances
"opensearch_number_of_shards" and "opensearch_number_of_replicas" currently
accept arbitrary strings; add input validation to ensure the shards value parses
to an integer >= 1 and the replicas value parses to an integer >= 0 before
saving. Implement this by wiring a validator/cleaner on these ConfigField
definitions (or the form save handler used by them) that attempts int(...)
conversion, rejects non-numeric input with a clear helper/error message, and
rejects values outside the allowed ranges so invalid inputs like "foo" or "-1"
cannot be persisted.

In `@src/tui/managers/env_manager.py`:
- Line 10: Remove the stale typing imports in src/tui/managers/env_manager.py:
drop Dict and List from the from typing import line (or remove the entire import
if Optional is also unused), and update any type annotations to use built-in
generics (e.g., dict, list) or keep only Optional if still referenced; ensure
the import line no longer includes Dict or List to resolve the UP035 Ruff error.

---

Outside diff comments:
In `@kubernetes/operator/internal/controller/env.go`:
- Line 34: Update the LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT allowlist to
include the new OpenSearch shard/replica env names so Langflow can read them at
runtime; specifically add OPENSEARCH_SHARDS and OPENSEARCH_REPLICAS to the
comma-separated string value of LANGFLOW_VARIABLES_TO_GET_FROM_ENVIRONMENT (the
same constant appears again further down and must be updated there as well) so
components/flows can access these variables.

In `@src/tui/managers/env_manager.py`:
- Around line 715-780: get_full_setup_fields() never includes the new OpenSearch
shard/replica environment keys, so guided setup keeps hard-coded 1/0 defaults;
add the two env keys (the exact names implemented in
EnvConfig/_env_attr_map/save_env_file) into the optional_fields list inside
get_full_setup_fields with user-friendly labels like "OpenSearch Index Shards"
and "OpenSearch Index Replicas" and sensible defaults ("1" and "0") and booleans
matching other optional items so the guided setup surfaces and saves those
values.

---

Nitpick comments:
In `@src/utils/embeddings.py`:
- Around line 63-70: The settings dict returned by the function (the block
returning {"settings": {...}, "mappings": {"properties": properties}}) places
number_of_shards and number_of_replicas at the top of settings, which is
inconsistent with other code that nests them under settings.index; update the
returned structure so that "settings.index" contains {"knn": True,
"number_of_shards": OPENSEARCH_NUMBER_OF_SHARDS, "number_of_replicas":
OPENSEARCH_NUMBER_OF_REPLICAS} (keep "mappings": {"properties": properties}
unchanged) and adjust any unit tests that assert
body["settings"]["number_of_shards"] to check
body["settings"]["index"]["number_of_shards"] instead.
🪄 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: 1d7b508d-0aa6-4c76-980a-60f358ffe4a7

📥 Commits

Reviewing files that changed from the base of the PR and between 17e8a19 and f0901d2.

📒 Files selected for processing (25)
  • .env.example
  • docker-compose.yml
  • docs/docs/reference/configuration.mdx
  • flows/components/opensearch_multimodal.py
  • flows/ingestion_flow.json
  • flows/openrag_agent.json
  • flows/openrag_nudges.json
  • flows/openrag_url_mcp.json
  • kubernetes/helm/openrag/templates/backend/backend-dotenv.yaml
  • kubernetes/helm/openrag/templates/langflow/langflow-dotenv.yaml
  • kubernetes/helm/openrag/values.yaml
  • kubernetes/operator/README.md
  • kubernetes/operator/api/v1alpha1/openrag_types.go
  • kubernetes/operator/config/crd/bases/openr.ag_openrags.yaml
  • kubernetes/operator/config/samples/kind-cluster-openrag-cr.yaml
  • kubernetes/operator/config/samples/openrag_v1alpha1_openrag.yaml
  • kubernetes/operator/internal/controller/env.go
  • kubernetes/operator/internal/controller/openrag_controller.go
  • kubernetes/operator/internal/controller/openrag_controller_test.go
  • src/config/settings.py
  • src/tui/config_fields.py
  • src/tui/managers/env_manager.py
  • src/utils/embeddings.py
  • src/utils/opensearch_init.py
  • tests/unit/test_embedding_fields.py


import copy
import json
import os

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove unused import.

The os module is imported but should not be used in this file per coding guidelines. Configuration values should be imported from config/settings.py instead.

As per coding guidelines, config values must come from config/settings.py (the only place os.environ is read); never access os.environ elsewhere in the codebase.

🤖 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 `@flows/components/opensearch_multimodal.py` at line 5, Remove the unused
"import os" from flows/components/opensearch_multimodal.py and instead import
any needed configuration values from config/settings.py (e.g., from
config.settings import SOME_SETTING) so no os.environ reads remain in this
module; update any local references that would have used os.environ to use the
corresponding settings symbols and ensure imports and names (e.g., any function
or class in this file that relied on env vars) are updated accordingly.

Comment on lines +34 to +43
def _get_min_env_int(key: str, default: int, minimum: int) -> int:
try:
value = int(os.getenv(key, default))
except (TypeError, ValueError):
value = default
return max(value, minimum)


OPENSEARCH_NUMBER_OF_SHARDS = _get_min_env_int("OPENRAG_OPENSEARCH_NUMBER_OF_SHARDS", 1, 1)
OPENSEARCH_NUMBER_OF_REPLICAS = _get_min_env_int("OPENRAG_OPENSEARCH_NUMBER_OF_REPLICAS", 0, 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Import shard/replica constants from config/settings.py instead of reading environment variables directly.

This code violates the coding guideline which states: "Config values must come from config/settings.py (the only place os.environ is read); never access os.environ elsewhere in the codebase."

The OPENSEARCH_NUMBER_OF_SHARDS and OPENSEARCH_NUMBER_OF_REPLICAS constants already exist in config/settings.py (lines 460-461) and should be imported from there.

♻️ Proposed fix to import from config/settings.py

Remove the duplicated helper and constant definitions:

-import os
-
-
-def _get_min_env_int(key: str, default: int, minimum: int) -> int:
-    try:
-        value = int(os.getenv(key, default))
-    except (TypeError, ValueError):
-        value = default
-    return max(value, minimum)
-
-
-OPENSEARCH_NUMBER_OF_SHARDS = _get_min_env_int("OPENRAG_OPENSEARCH_NUMBER_OF_SHARDS", 1, 1)
-OPENSEARCH_NUMBER_OF_REPLICAS = _get_min_env_int("OPENRAG_OPENSEARCH_NUMBER_OF_REPLICAS", 0, 0)

Add to the imports section near line 15 (where other config imports exist):

 from config.embedding_constants import OPENAI_DEFAULT_EMBEDDING_MODEL
+from config.settings import OPENSEARCH_NUMBER_OF_SHARDS, OPENSEARCH_NUMBER_OF_REPLICAS

As per coding guidelines, config values must come from config/settings.py (the only place os.environ is read); never access os.environ elsewhere in the codebase.

🤖 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 `@flows/components/opensearch_multimodal.py` around lines 34 - 43, Remove the
local _get_min_env_int helper and the OPENSEARCH_NUMBER_OF_SHARDS /
OPENSEARCH_NUMBER_OF_REPLICAS constant definitions and instead import the
existing constants from config.settings; update the imports near the other
config imports (around line where other config values are imported) to pull
OPENSEARCH_NUMBER_OF_SHARDS and OPENSEARCH_NUMBER_OF_REPLICAS from
config.settings, then delete the _get_min_env_int function and the two local
constant assignments in flows/components/opensearch_multimodal.py so all config
values come from config/settings.py as required.

Comment thread src/tui/config_fields.py
from collections.abc import Callable
from dataclasses import dataclass
from dataclasses import field as dataclass_field
from typing import Optional

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove the unused Optional import.

Ruff is already failing on Line 10 for this.

🧰 Tools
🪛 GitHub Actions: Lint Backend / 0_Ruff and mypy on changed files.txt

[error] 10-10: ruff check --no-fix failed (F401). typing.Optional imported but unused

🪛 GitHub Actions: Lint Backend / Ruff and mypy on changed files

[error] 10-10: Ruff (ruff check --no-fix) lint error: F401 typing.Optional imported but unused

🪛 GitHub Check: Ruff and mypy on changed files

[failure] 10-10: ruff (F401)
src/tui/config_fields.py:10:20: F401 typing.Optional imported but unused
help: Remove unused import: typing.Optional

🤖 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/tui/config_fields.py` at line 10, Remove the unused import Optional from
the import statement (current line imports "Optional") in config_fields.py;
update the top-level typing import so it no longer imports Optional (leaving
only used names) to satisfy Ruff and eliminate the unused-import error.

Comment thread src/tui/config_fields.py
Comment on lines +125 to +142
ConfigField(
"opensearch_number_of_shards",
"OPENRAG_OPENSEARCH_NUMBER_OF_SHARDS",
"Primary Shards",
placeholder="1",
default="1",
advanced=True,
helper_text="Primary shard count for newly-created OpenRAG indices",
),
ConfigField(
"opensearch_number_of_replicas",
"OPENRAG_OPENSEARCH_NUMBER_OF_REPLICAS",
"Replica Shards",
placeholder="0",
default="0",
advanced=True,
helper_text="Replica shard count for OpenRAG indices",
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate shard/replica inputs before saving them.

These fields map to CRD-backed settings with minimums of shards >= 1 and replicas >= 0, but the TUI currently accepts arbitrary strings here. That lets users save values like foo or -1 and only discover the problem later during startup/reconciliation.

🤖 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/tui/config_fields.py` around lines 125 - 142, The two ConfigField
instances "opensearch_number_of_shards" and "opensearch_number_of_replicas"
currently accept arbitrary strings; add input validation to ensure the shards
value parses to an integer >= 1 and the replicas value parses to an integer >= 0
before saving. Implement this by wiring a validator/cleaner on these ConfigField
definitions (or the form save handler used by them) that attempts int(...)
conversion, rejects non-numeric input with a clear helper/error message, and
rejects values outside the allowed ranges so invalid inputs like "foo" or "-1"
cannot be persisted.

@@ -10,6 +10,7 @@
from typing import Dict, List, Optional

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Drop the stale typing imports.

After switching this file to built-in generics, Dict/List are just leftover imports, and Ruff is already failing with UP035 here.

🧰 Tools
🪛 GitHub Actions: autofix.ci / 0_ruff autofix.txt

[error] 10-10: Ruff (UP035): typing.Dict is deprecated; use dict instead.


[error] 10-10: Ruff (UP035): typing.List is deprecated; use list instead.

🪛 GitHub Actions: autofix.ci / ruff autofix

[error] 10-10: ruff check failed (exit code 1). UP035: typing.Dict is deprecated, use dict instead.


[error] 10-10: ruff check failed (exit code 1). UP035: typing.List is deprecated, use list instead.

🪛 GitHub Check: Ruff and mypy on changed files

[failure] 10-10: ruff (F401)
src/tui/managers/env_manager.py:10:32: F401 typing.Optional imported but unused
help: Remove unused import


[failure] 10-10: ruff (F401)
src/tui/managers/env_manager.py:10:26: F401 typing.List imported but unused
help: Remove unused import


[failure] 10-10: ruff (F401)
src/tui/managers/env_manager.py:10:20: F401 typing.Dict imported but unused
help: Remove unused import


[failure] 10-10: ruff (UP035)
src/tui/managers/env_manager.py:10:1: UP035 typing.List is deprecated, use list instead


[failure] 10-10: ruff (UP035)
src/tui/managers/env_manager.py:10:1: UP035 typing.Dict is deprecated, use dict instead

🤖 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/tui/managers/env_manager.py` at line 10, Remove the stale typing imports
in src/tui/managers/env_manager.py: drop Dict and List from the from typing
import line (or remove the entire import if Optional is also unused), and update
any type annotations to use built-in generics (e.g., dict, list) or keep only
Optional if still referenced; ensure the import line no longer includes Dict or
List to resolve the UP035 Ruff error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) docker enhancement 🔵 New feature or request tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant