[WEB-4720] fix: mongo connection class to initialize mongo db #7652
Conversation
WalkthroughAdds MongoDB environment configuration variables in Django settings and updates the Mongo settings helper so is_configured ensures a singleton instance is created when missing before checking connectivity. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant MongoSettings as MongoSettings (class)
Caller->>MongoSettings: is_configured()
alt _client is None
MongoSettings->>MongoSettings: _instance = cls()<br/>(instantiate singleton)
else _client exists
Note over MongoSettings: Use existing client
end
MongoSettings->>MongoSettings: Check _client and _db
MongoSettings-->>Caller: return (_client != None && _db != None)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the MongoDB connection class where the is_configured method would incorrectly return False even when MongoDB was properly configured, because it wasn't initializing the connection before checking the status. The fix ensures the MongoDB connection is established when checking configuration status.
- Fix MongoDB connection initialization in the
is_configuredclass method - Add MongoDB environment variables to Django settings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/api/plane/settings/mongo.py | Adds initialization logic to ensure MongoDB connection is established before status check |
| apps/api/plane/settings/common.py | Adds MongoDB URL and database environment variable configurations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if cls._client is None: | ||
| cls._instance = cls() |
There was a problem hiding this comment.
The code sets cls._instance but doesn't assign the result to cls._client or cls._db. This means the initialization won't actually populate the client/database attributes that are checked in the return statement. You should either call a method that properly initializes these attributes or ensure the constructor properly sets cls._client and cls._db.
| if cls._client is None: | |
| cls._instance = cls() | |
| if cls._client is None or cls._db is None: | |
| cls._initialize_connection() |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/api/plane/settings/common.py (2)
470-471: Prefer None defaults to avoid mixing bool and str typesStaying type-consistent helps downstream checks and tooling. Current truthiness checks still work, but None is a clearer “unset” sentinel.
Apply this minimal diff:
-MONGO_DB_URL = os.environ.get("MONGO_DB_URL", False) -MONGO_DB_DATABASE = os.environ.get("MONGO_DB_DATABASE", False) +MONGO_DB_URL = os.environ.get("MONGO_DB_URL") +MONGO_DB_DATABASE = os.environ.get("MONGO_DB_DATABASE")Optional alternative for broader compatibility (commonly used var names):
-MONGO_DB_URL = os.environ.get("MONGO_DB_URL", False) +MONGO_DB_URL = os.environ.get("MONGO_DB_URL") or os.environ.get("MONGODB_URI") -MONGO_DB_DATABASE = os.environ.get("MONGO_DB_DATABASE", False) +MONGO_DB_DATABASE = os.environ.get("MONGO_DB_DATABASE") or os.environ.get("MONGODB_DATABASE")
469-471: Document new env vars and sample defaultsAdd these to your env templates/docs (e.g., .env.example, Helm/Compose values) so operators know how to enable Mongo.
I can open a follow-up PR to add doc snippets and sample values if you’d like.
apps/api/plane/settings/mongo.py (2)
122-124: Also re-init when _db is None to avoid partial stateIf initialization fails to set the db (e.g., missing database name) but _client ends up non-None, you won’t retry. Light tweak ensures a retry path.
Apply this diff:
- if cls._client is None: + if cls._client is None or cls._db is None: cls._instance = cls()
122-124: Add timeout + ping and make init thread-safe (optional hardening)
- Connectivity probe: prefer client.admin.command("ping") with a short serverSelectionTimeoutMS to avoid long blocking on first access.
- Thread safety: guard singleton initialization with a lock to prevent duplicate inits under load.
- Logging: consider sanitizing exception text to avoid leaking parts of the URI from driver errors.
Outside the changed lines, the following pattern keeps behavior but improves resilience:
# at top of module import threading # inside MongoConnection class _lock = threading.Lock() def __new__(cls: Type[T]) -> T: if cls._instance is None: with cls._lock: if cls._instance is None: # double-checked cls._instance = super(MongoConnection, cls).__new__(cls) try: mongo_url = getattr(settings, "MONGO_DB_URL", None) mongo_db_database = getattr(settings, "MONGO_DB_DATABASE", None) if not mongo_url or not mongo_db_database: logger.warning("MongoDB not configured; functionality disabled.") return cls._instance cls._client = MongoClient( mongo_url, serverSelectionTimeoutMS=int(os.environ.get("MONGO_DB_TIMEOUT_MS", "2000")), connectTimeoutMS=int(os.environ.get("MONGO_DB_CONNECT_TIMEOUT_MS", "2000")), ) cls._db = cls._client[mongo_db_database] # Fast, low-privilege health check cls._client.admin.command("ping") logger.info("MongoDB connection established successfully") except Exception as e: # Avoid printing full URI; log class and reason logger.warning(f"Failed to initialize MongoDB connection: {e.__class__.__name__}: {e}") return cls._instance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/plane/settings/common.py(1 hunks)apps/api/plane/settings/mongo.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/api/plane/settings/common.py (1)
469-471: MongoDB settings surfaced via env: LGTMAdding explicit MONGO_DB_URL and MONGO_DB_DATABASE knobs keeps configuration centralized and discoverable.
apps/api/plane/settings/mongo.py (2)
122-124: Ensuring init on is_configured(): LGTMTriggering singleton initialization from is_configured aligns with the PR intent and unblocks connectivity checks.
122-124: Add targeted unit tests for MongoConnection and confirm caller resilience
Call-site audit:
•apps/api/plane/bgtasks/cleanup_task.py(lines 35–43) already checksMongoConnection.is_configured()before callingget_collection().
• No other references toMongoConnection.(is_configured|get_db|get_client|get_collection)found.
• No directMongoClient(...)instantiations outside ofapps/api/plane/settings/mongo.py.Testing requirements:
• MockMongoClientto assertis_configured()returns:
–FalsewhenMONGO_DB_URLorMONGO_DB_DATABASEenv vars are unset.
–Truewhen env vars are set andMongoClient().ping()succeeds.
–Falsewhen env vars are set butMongoClient().ping()throws an error.
• Coverget_client(),get_db(), andget_collection()for both configured and error paths.
• Verify consumers (e.g., the cleanup task) handleis_configured() == Falsewithout assuming a DB is available.
Description
is_configuredis called.Type of Change
Test Scenarios
References
WEB-4720
Summary by CodeRabbit