Add TTL index creation for runs collection in state-manager( Issue #432)#565
Add TTL index creation for runs collection in state-manager( Issue #432)#565Brijesh-Thakkar wants to merge 6 commits intoexospherehost:mainfrom
Conversation
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds Docker Compose for state manager and dashboard, a .env example and README updates, introduces TTL index management during state-manager startup, adds pydantic-settings and a Node mongodb dependency, and updates .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (docker-compose)
participant Dash as Exosphere Dashboard
participant SM as Exosphere State Manager
participant DB as MongoDB
Dev->>Dash: Start services (docker-compose)
Dev->>SM: Start services (docker-compose)
Dash->>SM: Wait for health (depends_on condition)
SM->>DB: Initialize DB connection (beanie / pymongo)
SM->>DB: ensure_ttl_indexes(collections=["runs"], ttl_days=RUN_TTL_DAYS)
alt TTL index exists with correct TTL
DB-->>SM: OK
else TTL missing or mismatch
DB-->>SM: create/collMod index → result
end
SM-->>Dash: Become healthy
Dash->>SM: API requests (runtime)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
Summary of ChangesHello @Brijesh-Thakkar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial database maintenance feature by adding a Time-To-Live (TTL) index to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a TTL (Time-To-Live) index on the runs collection to automatically delete old documents, which is a great way to prevent database bloat. The implementation in state-manager/app/main.py is well-structured. My feedback focuses on improving configuration and reusability by avoiding hardcoded values. Additionally, I've pointed out a security concern in the new docker-compose.yml regarding default secrets, which should be addressed. Overall, this is a valuable addition for maintaining the health of the database.
docker-compose.yml
Outdated
| - STATE_MANAGER_SECRET=${STATE_MANAGER_SECRET:-exosphere@123} | ||
| - MONGO_DATABASE_NAME=${MONGO_DATABASE_NAME:-exosphere} | ||
| - SECRETS_ENCRYPTION_KEY=${SECRETS_ENCRYPTION_KEY:-YTzpUlBGLSwm-3yKJRJTZnb0_aQuQQHyz64s8qAERVU=} |
There was a problem hiding this comment.
Committing default secrets, even for local development, is a security risk. These secrets could be inadvertently used in other environments. It's recommended to remove the default values. For local development, you can use a .env file (which should be in .gitignore) and provide a .env.example file with placeholder values to guide users.
- STATE_MANAGER_SECRET=${STATE_MANAGER_SECRET}
- MONGO_DATABASE_NAME=${MONGO_DATABASE_NAME:-exosphere}
- SECRETS_ENCRYPTION_KEY=${SECRETS_ENCRYPTION_KEY}
docker-compose.yml
Outdated
| environment: | ||
| # Server-side secure configuration (NOT exposed to browser) | ||
| - EXOSPHERE_STATE_MANAGER_URI=${EXOSPHERE_STATE_MANAGER_URI:-http://exosphere-state-manager:8000} | ||
| - EXOSPHERE_API_KEY=${EXOSPHERE_API_KEY:-exosphere@123} |
There was a problem hiding this comment.
Similar to the other secrets, this default API key should be removed to avoid security risks. It's better to rely on environment variables provided via a .env file for local setup.
- EXOSPHERE_API_KEY=${EXOSPHERE_API_KEY}
state-manager/app/main.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| async def ensure_ttl_indexes(db, ttl_days: int = 30, collections: Optional[List[str]] = None): |
There was a problem hiding this comment.
For better type safety and code clarity, consider adding a type hint for the db parameter. You would need to import Database from pymongo.database and then change the signature to async def ensure_ttl_indexes(db: Database, ...).
state-manager/app/main.py
Outdated
| collections = ["runs"] | ||
|
|
||
| ttl_seconds = int(ttl_days) * 24 * 3600 | ||
| timestamp_field = "created_at" |
There was a problem hiding this comment.
The timestamp_field is hardcoded to "created_at". This makes the function less reusable if you want to apply TTL indexes on other collections with different timestamp fields in the future. Consider making this a parameter of the ensure_ttl_indexes function, e.g., timestamp_field: str = "created_at".
state-manager/app/main.py
Outdated
| logger = LogsManager().get_logger() | ||
| logger.info("server starting") | ||
| # beginning of the server | ||
| log = LogsManager().get_logger() |
There was a problem hiding this comment.
You've introduced a module-level logger on line 53, which is great practice. However, inside the lifespan function, you are getting another logger instance via LogsManager().get_logger() and assigning it to log. This is inconsistent. For consistency, you should use the module-level logger throughout the lifespan function as well.
state-manager/app/main.py
Outdated
| # Start with 'runs' only to be safe. Add other collections once you confirm. | ||
| log.info("Starting TTL index creation...") | ||
| try: | ||
| await ensure_ttl_indexes(db, ttl_days=30, collections=["runs"]) |
There was a problem hiding this comment.
The TTL value of 30 days is hardcoded. It's better to make this configurable via environment variables and read it from settings, similar to trigger_retention_hours. This would allow changing the retention period without code changes. For example, you could add a RUN_RETENTION_DAYS setting.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
docker-compose.yml(1 hunks)package.json(1 hunks)state-manager/app/main.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
state-manager/app/main.py (4)
state-manager/app/tasks/init_tasks.py (1)
init_tasks(16-20)state-manager/app/models/db/trigger.py (1)
DatabaseTriggers(9-52)state-manager/app/config/settings.py (1)
get_settings(34-38)state-manager/app/utils/check_database_health.py (1)
check_database_health(3-19)
🔇 Additional comments (7)
docker-compose.yml (2)
18-23: Healthcheck configurations look good.The healthchecks have appropriate intervals, timeouts, retries, and start periods for both services. The dashboard's dependency on
service_healthyensures proper startup ordering.Also applies to: 43-48
50-53: Network configuration is appropriate.The bridge network with
attachable: trueallows additional containers to join if needed, which is useful for debugging and development.state-manager/app/main.py (4)
126-131: Good resilient error handling for TTL setup.The approach of logging errors but continuing startup is appropriate—TTL index failures shouldn't prevent the application from serving requests. The
log.exceptioncaptures the full traceback for debugging.
9-10: Imports look appropriate for the new functionality.The
loggingandtypingimports support the newensure_ttl_indexesfunction properly.
183-189: Helpful middleware ordering documentation.The comments clearly explain the execution order, which is valuable for maintainability.
72-72: Verify that theRunmodel has acreated_atfield.The TTL index relies on
created_atexisting on documents. If this field is missing or named differently, documents won't be expired.package.json (1)
1-5: Clarify the purpose and location of this package.json.The state-manager is a Python application using
pymongo, not the Node.js MongoDB driver. Thispackage.jsonat the repository root seems out of place. If it's for the dashboard service, it should be in the dashboard's directory. If this serves a different purpose (e.g., workspace root configuration, separate tooling), add documentation explaining its role.
state-manager/app/main.py
Outdated
| # Start with 'runs' only to be safe. Add other collections once you confirm. | ||
| log.info("Starting TTL index creation...") | ||
| try: | ||
| await ensure_ttl_indexes(db, ttl_days=30, collections=["runs"]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make TTL days configurable via settings.
The 30-day TTL is hardcoded. For operational flexibility, consider making this configurable through environment variables similar to other settings.
In settings.py, add:
ttl_days: int = Field(default=30, description="TTL in days for runs collection")Then use it here:
- await ensure_ttl_indexes(db, ttl_days=30, collections=["runs"])
+ await ensure_ttl_indexes(db, ttl_days=settings.ttl_days, collections=["runs"])🤖 Prompt for AI Agents
In state-manager/app/main.py around line 127, the call to ensure_ttl_indexes
uses a hardcoded ttl_days=30; change it to use the TTL setting from settings
(e.g., settings.ttl_days). Add the new Field ttl_days:int with default 30 in
settings.py as suggested, import or access the settings object in main.py, and
pass settings.ttl_days into ensure_ttl_indexes so the TTL becomes configurable
via environment-backed settings.
…; update config & compose
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
state-manager/app/config/settings.py (1)
17-28: Validatettl_daysto guard against invalid TTL configurationThe new
ttl_dayssetting is wired correctly and matches the compose default, but currently any integer (including 0 or negative) fromTTL_DAYSwill be accepted and passed into TTL index creation. That can cause index creation/update errors or immediate expiry.Consider enforcing a sensible lower bound, for example:
ttl_days: int = Field(gt=0, default=30, description="TTL in days for TTL indexes")and/or validating the parsed value in
from_envbefore returningSettings, so bad configuration fails fast instead of only surfacing via TTL index errors at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)README.md(2 hunks)docker-compose.yml(1 hunks)state-manager/app/config/settings.py(2 hunks)state-manager/app/main.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
state-manager/app/main.py (7)
state-manager/app/tasks/trigger_cron.py (1)
trigger_cron(97-101)state-manager/app/tasks/init_tasks.py (1)
init_tasks(16-20)state-manager/app/models/db/trigger.py (1)
DatabaseTriggers(9-52)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)state-manager/app/config/settings.py (1)
get_settings(36-40)state-manager/app/middlewares/unhandled_exceptions_middleware.py (1)
UnhandledExceptionsMiddleware(10-30)state-manager/app/middlewares/request_id_middleware.py (1)
RequestIdMiddleware(10-54)
🔇 Additional comments (6)
docker-compose.yml (2)
9-14: Secrets handling in compose looks good nowUsing
${VAR:?VAR must be set}forMONGO_URI,STATE_MANAGER_SECRET,SECRETS_ENCRYPTION_KEY, andEXOSPHERE_API_KEYremoves insecure defaults and forces explicit configuration via.envor environment, which matches the new README guidance. This resolves the earlier security concerns about baked‑in secrets.Also applies to: 31-37
12-13: Unify defaultMONGO_DATABASE_NAMEbetween compose and settingsThe container env var specifies
MONGO_DATABASE_NAME=${MONGO_DATABASE_NAME:-exosphere}, whileSettings.from_env()instate-manager/app/config/settings.pymay use a different default. If these defaults differ, compose-based runs and local/non-compose runs will behave differently. Consider standardizing on a single default value across both configuration sources.README.md (2)
238-256: Environment and secrets setup docs are clear and aligned with the compose configThe new step‑by‑step
.envworkflow (copying.env.example, generating strong secrets, and the explicit security note) lines up well with the updateddocker-compose.ymlthat no longer bakes in default secrets. This should significantly reduce the risk of accidental weak credentials in local or production deployments.Also applies to: 270-274, 280-280
259-268: No action needed — compose file paths are correctThe docker-compose files are confirmed to exist in the
docker-compose/directory on the main branch, matching the URLs in the README. The curl commands on lines 259-268 correctly reference:
docker-compose/docker-compose.ymldocker-compose/docker-compose-with-mongodb.ymlNo path adjustments are required.
Likely an incorrect or invalid review comment.
state-manager/app/main.py (2)
56-174: TTL index helper is robust and matches the new configuration modelThe
ensure_ttl_indexeshelper cleanly encapsulates TTL setup:
- Defaults to the
runscollection andcreated_atfield, but allows overriding collections and the timestamp field.- Computes
expireAfterSecondsfromttl_days, checks existing indexes by name, and:
- Uses
collModto update TTL when the value changes.- Falls back to drop+recreate on
collModfailure.- Logs per-collection success/failure and completes without crashing the server, aligning with a best-effort indexing strategy.
Combined with
settings.ttl_daysand the newTTL_DAYSenv wiring, this should keep therunscollection under control while staying flexible for future collections or fields.
192-202: Decide whether TTL index creation failures should be fatal
ensure_ttl_indexes(...)is wrapped in a try-except that logs errors but allows the app to continue. If TTL creation fails (e.g., badTTL_DAYS, insufficient privileges), the app starts without TTL enforcement, potentially allowing the 'runs' collection to grow unbounded—contrary to this PR's stated goal.Consider either:
- Failing fast (re-raising) when running in production, or
- Making this configurable (e.g.,
FAIL_ON_TTL_SETUP_ERRORsetting)At minimum, log clearly whether TTL enforcement is active or was skipped.
…pe hints, structured logging
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)docker-compose.yml(1 hunks)state-manager/app/config/settings.py(2 hunks)state-manager/app/main.py(4 hunks)
🔇 Additional comments (5)
state-manager/app/main.py (4)
221-225: Lifespan logging and shutdown sequencing look goodThe added lifecycle logs (
server_starting,beanie_initialized,init_tasks_*,secret_initialized,server_stopped) and the explicit secret check plus scheduler wiring make startup/shutdown behavior easier to observe and reason about. The order—init DB/Béanie → TTL setup → init_tasks → secret validation → health check → cron scheduling → yield → DB close + scheduler shutdown—is coherent and should be operationally friendly.Also applies to: 227-231, 235-244, 250-253
285-286: Router inclusion order is fineIncluding
global_routerbefore the mainrouteris a reasonable choice and avoids any surprise overrides; no issues here.
8-10: Async PyMongo types and methods are correctly used for PyMongo 4.0+The imports
AsyncMongoClientandAsyncDatabasefrompymongo.asynchronous.databaseare correct for modern PyMongo versions (4.0+). The methodsdb.command(),collection.index_information(),collection.create_index(), andcollection.drop_index()are all awaitable in the PyMongo async API.However, verify that:
- Your project depends on PyMongo 4.0 or later (not Motor, which is deprecated),
- All usages of these methods at lines 56–61 and 201–207 properly include
awaitstatements,- The client is initialized with
AsyncMongoClientand not accidentally synchronous, and- The database and collection instances are obtained from the async client (not mixed with sync clients).
56-61: TTL index management is robust; be explicit about limitations and env namingThe
ensure_ttl_indexesflow (check existing index, collMod on TTL mismatch, drop+recreate on failure, and structured logging) is a solid, defensive implementation and will handle most operational cases well.Two small follow‑ups to consider:
Existing TTL indexes with a different name/key pattern: MongoDB's
collModcannot modify an index's keyPattern—it only updatesexpireAfterSecondson an existing index. If an earlier version created a TTL index with a different name or key pattern, this code's approach of creating a newttl_created_at_indexand relying onIndexOptionsConflicthandling is appropriate. However, if you expect pre‑existing TTL indexes in production, document this conflict behavior clearly or consider a one‑time migration path so operators understand what happens when index names or patterns differ.Env naming in comments: verify that the lifespan comment accurately reflects the actual environment variable precedence (particularly whether
RUN_TTL_DAYSorTTL_DAYSis the primary override) and that it matches the current docker‑compose configuration, so operators know which variable to set.Behaviorally, the "log and continue" strategy on TTL failures is reasonable for avoiding startup breaks; if you ever need a stricter mode, a future
TTL_FAIL_FAST/similar flag could toggle between "warn and continue" vs "raise".docker-compose.yml (1)
1-27: Compose service config and secret handling look solidMoving secrets to required env vars with
${VAR:?...}and pointing users at.env/.env.exampleis a good security improvement.TTL_DAYSwith a default of 30 aligns withrun_ttl_days'sTTL_DAYSfallback inSettings.from_env, so operators can tune TTL per deployment without code changes.If you want to surface the more granular
RUN_TTL_DAYSin compose as well (for future per‑collection TTLs), you could add it alongsideTTL_DAYS, but the current setup already works correctly with the fallback chain.
| ttl_days: int = Field(default=30, description="TTL in days for TTL indexes") | ||
| # Specific TTL for runs collection; overrides ttl_days when used for runs | ||
| run_ttl_days: int = Field(30, env="RUN_TTL_DAYS", description="TTL in days for runs collection") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
TTL settings and env fallbacks look correct; optional Pydantic simplification
ttl_days and run_ttl_days with the RUN_TTL_DAYS -> TTL_DAYS -> 30 precedence match the intended flexibility and line up with the compose defaults. The direct int(os.getenv(...)) parsing also matches how trigger_workers and trigger_retention_hours are handled.
If you later consolidate config handling, consider letting Pydantic drive env parsing (e.g., using env="TTL_DAYS" / env="RUN_TTL_DAYS" plus a custom @classmethod or validator for fallback) to centralize defaults and get more consistent error messages on bad env values, but current behavior is fine.
Also applies to: 29-31
🤖 Prompt for AI Agents
In state-manager/app/config/settings.py around lines 17-19 and 29-31,
consolidate TTL env parsing into Pydantic-driven logic: declare both fields with
env names (env="TTL_DAYS" and env="RUN_TTL_DAYS") and implement a validator or
root_validator that applies the precedence RUN_TTL_DAYS -> TTL_DAYS -> 30,
converting and validating values as ints; this centralizes parsing, avoids
manual os.getenv int casts elsewhere, and ensures clear validation errors when
env values are invalid.
…ddleware comments
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.env.example(1 hunks).gitignore(1 hunks)docker-compose.yml(1 hunks)state-manager/app/main.py(4 hunks)state-manager/pyproject.toml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
state-manager/app/main.py (6)
state-manager/app/models/db/run.py (1)
Run(7-25)state-manager/app/singletons/logs_manager.py (2)
LogsManager(9-66)get_logger(65-66)state-manager/app/config/settings.py (1)
get_settings(39-43)state-manager/app/utils/check_database_health.py (1)
check_database_health(3-19)state-manager/app/middlewares/request_id_middleware.py (1)
RequestIdMiddleware(10-54)state-manager/app/middlewares/unhandled_exceptions_middleware.py (1)
UnhandledExceptionsMiddleware(10-30)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 25-25: [UnorderedKey] The RUN_TTL_DAYS key should go before the TTL_DAYS key
(UnorderedKey)
🔇 Additional comments (17)
state-manager/pyproject.toml (1)
21-21: LGTM! Dependency addition supports environment-based TTL configuration.The
pydantic-settings>=2.0.0dependency is necessary for the new TTL-related settings fields and environment loading introduced in this PR..gitignore (1)
1-26: Comprehensive ignore patterns are well-scoped.The ignore patterns appropriately cover Node modules, environment files, Python caches, IDE folders, and logs. The explicit comment about keeping
.env.exampletracked is helpful..env.example (4)
1-4: Clear security guidance in template header.The header appropriately warns users to never commit real secrets and provides clear instructions for local usage.
5-9: MongoDB configuration examples are clear and helpful.The MongoDB connection string example and database name default align with the docker-compose configuration.
11-20: Secret placeholders with helpful generation guidance.The placeholder values ("changeme") are safe for a template, and the
openssl rand -hex 32examples provide clear guidance for generating secure values.
22-25: TTL configuration with sensible defaults and clear override explanation.The 30-day default and the explanation of how
RUN_TTL_DAYSoverridesTTL_DAYSfor runs-specific TTL are clear and appropriate.state-manager/app/main.py (6)
8-10: Correct async type hints for MongoDB usage.The imports properly use
AsyncDatabasefrompymongo.asynchronous.database, which aligns with the async/await patterns used throughout the code.
52-53: Consistent structured logging setup.The module-level logger now uses
LogsManagerfor consistent structured logging across the application, addressing previous inconsistencies.
56-74: Well-documented TTL index management function.The function signature is properly typed with
AsyncDatabase, and the comprehensive docstring clearly explains the purpose, parameters, and behavior including the collMod update strategy.
78-191: Robust TTL index management with proper fallback handling.The implementation correctly:
- Detects existing indexes and compares TTL values
- Updates via
collModwhen TTL differs- Falls back to drop/recreate if
collModfails- Uses structured logging with key-value pairs throughout
- Handles errors per collection without crashing the server
The logic addresses all edge cases and aligns with MongoDB best practices.
209-220: TTL index setup properly integrated into application lifecycle.The TTL setup is conservatively placed after Beanie initialization but before init_tasks, uses configurable
settings.run_ttl_days, and includes appropriate error handling that logs failures without crashing the server. The structured logging provides good visibility into the initialization process.
271-277: Middleware ordering is correct with clear documentation.The registration order properly makes
UnhandledExceptionsMiddlewarethe outermost layer, allowing it to catch exceptions from bothRequestIdMiddlewareandCORSMiddleware. The comment clearly explains the inner-to-outer ordering.docker-compose.yml (5)
1-10: Clear security guidance and proper local build configuration.The security warning appropriately directs users to
.envfor local secrets, and the build configuration correctly uses the local state-manager context.
21-26: Health check properly configured with curl.The health check correctly uses
curl -fagainst the/healthendpoint with reasonable timing parameters.
28-45: Dashboard service properly configured with health dependency.The dashboard configuration correctly:
- Uses the published image with configurable tag
- Requires
EXOSPHERE_API_KEYvia:?syntax- Depends on state-manager being healthy before starting
- Separates server-side and client-side environment variables with clear comments
46-51: Dashboard health check correctly uses curl.The health check properly uses
curl -fto verify the dashboard is responding on port 3000.
53-56: Network configuration is appropriate for local development.The bridge network with attachable flag allows both services to communicate and enables manual attachment of other containers if needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docker-compose.yml(1 hunks)
🔇 Additional comments (3)
docker-compose.yml (3)
12-28: Environment and healthcheck configuration for state-manager looks good overall.The required secrets use
?MUST_SETsyntax (lines 12–15), TTL configuration is present (lines 16–17), and the healthcheck uses portablecurlwith adequate timing for TTL index creation (lines 23–28). Assuming the nested variable substitution issue on line 17 is resolved, the state-manager service is well-configured for the TTL feature.
30-53: Dashboard service configuration is sound.The dashboard correctly depends on state-manager with
service_healthycondition (lines 41–43), ensuring the state-manager's TTL setup completes before the dashboard connects. The healthcheck uses portablecurl(line 49), and environment variables are properly scoped (server-side vs. client-side, lines 36–40).
1-58: Note: No MongoDB service in compose — verify this is intentional for this PR.The compose file does not include a MongoDB service; instead, it expects an external MongoDB via
MONGO_URI(line 12). If the intent of this PR is to provide a complete local development setup with TTL indexing, consider whether amongoservice should be included. If MongoDB is intentionally managed separately, ensure this is documented in setup instructions.
docker-compose.yml
Outdated
| - TTL_DAYS=${TTL_DAYS:-30} | ||
| - RUN_TTL_DAYS=${RUN_TTL_DAYS:-${TTL_DAYS:-30}} |
There was a problem hiding this comment.
Fix nested variable substitution syntax — RUN_TTL_DAYS fallback will not work as intended.
Docker Compose does not support nested variable expansion. The current syntax:
- RUN_TTL_DAYS=${RUN_TTL_DAYS:-${TTL_DAYS:-30}}If RUN_TTL_DAYS is unset, this will literally set RUN_TTL_DAYS to the string "${TTL_DAYS:-30}" instead of evaluating it to TTL_DAYS or 30. This breaks the intended fallback logic for the TTL feature.
Simplify to:
- RUN_TTL_DAYS=${RUN_TTL_DAYS}Then ensure that upstream .env or deployment automation explicitly sets RUN_TTL_DAYS (or omit it entirely if the state-manager defaults suffice). Alternatively, if you need a per-compose default, set it at the top level of the compose file or document the .env requirement clearly.
🤖 Prompt for AI Agents
In docker-compose.yml around lines 16-17, the RUN_TTL_DAYS environment line uses
unsupported nested variable expansion which will set the literal string
"${TTL_DAYS:-30}" when RUN_TTL_DAYS is unset; replace the nested fallback with a
single variable reference (e.g. set RUN_TTL_DAYS=${RUN_TTL_DAYS}) and ensure the
desired default is provided either via the top-level compose "env_file"/".env"
or by the deployment automation, or document that RUN_TTL_DAYS must be set so
the service receives the correct TTL value.
|
I cannot merge this PR: |
2 participants
Added ensure_ttl_creation function
To prevent databse bloat and improve state manager performance
It is currently apptied to only "runs"