Depreciated code and warning fix #140
Conversation
WalkthroughConsolidates environment-driven settings into a new backend/config.py and updates multiple GitHub devrel and embedding services to read from it. Migrates several Pydantic models/settings to v2-style model_config using ConfigDict. No public API changes; logic and endpoints remain the same. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![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
🧹 Nitpick comments (3)
backend/app/agents/devrel/github/services/github_mcp_service.py (1)
8-8: Annotate the optional token explicitlyLint is flagging the implicit optional. Switch to an explicit union (or
Optional[str]) to satisfy PEP 484-style typing.- def __init__(self, token: str = None): + def __init__(self, token: str | None = None):backend/config.py (2)
5-9: Simplify dotenv loading.The conditional loading pattern is correct but more verbose than necessary.
load_dotenv()already callsfind_dotenv()internally and handles missing files gracefully.Apply this diff to simplify:
-dotenv_path = find_dotenv(usecwd=True) -if dotenv_path: - load_dotenv(dotenv_path=dotenv_path) -else: - load_dotenv() +# load_dotenv searches for .env upward from the current directory +load_dotenv(find_dotenv(usecwd=True))Based on learnings
1-16: Consider adding module docstring and configuration documentation.This is now the central configuration module for the project. Consider adding:
- Module-level docstring explaining purpose and expected .env location
- Comments documenting each configuration variable's purpose and valid values
- Type hints for the exported constants
Example:
""" Central configuration module for Devr.AI backend. Loads environment variables from .env file in the project root or parent directories. Falls back to default values when environment variables are not set. Configuration variables: - GITHUB_ORG: GitHub organization name (default: "Aossie-org") - GITHUB_TOKEN: GitHub API token (also checks GH_TOKEN) - MODEL_NAME: Embedding model identifier (default: "BAAI/bge-small-en-v1.5") - MAX_BATCH_SIZE: Maximum batch size for embeddings (default: 32) - EMBEDDING_DEVICE: Device for embeddings - "cpu" or "cuda" (default: "cpu") """ from dotenv import load_dotenv, find_dotenv import os # Load environment variables load_dotenv(find_dotenv(usecwd=True)) # GitHub configuration GITHUB_ORG: str = os.getenv("GITHUB_ORG", "Aossie-org") GITHUB_TOKEN: str | None = os.getenv("GITHUB_TOKEN") or os.getenv("GH_TOKEN") # Embedding service configuration MODEL_NAME: str = os.getenv("EMBEDDING_MODEL", "BAAI/bge-small-en-v1.5") try: MAX_BATCH_SIZE: int = int(os.getenv("EMBEDDING_MAX_BATCH_SIZE", "32")) except ValueError: raise ValueError( "EMBEDDING_MAX_BATCH_SIZE must be a valid integer. " f"Got: {os.getenv('EMBEDDING_MAX_BATCH_SIZE')}" ) EMBEDDING_DEVICE: str = os.getenv("EMBEDDING_DEVICE", "cpu")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/app/agents/devrel/github/github_toolkit.py(2 hunks)backend/app/agents/devrel/github/services/github_mcp_client.py(2 hunks)backend/app/agents/devrel/github/services/github_mcp_server.py(1 hunks)backend/app/agents/devrel/github/services/github_mcp_service.py(2 hunks)backend/app/agents/devrel/github/tools/github_support.py(1 hunks)backend/app/agents/state.py(2 hunks)backend/app/core/config/settings.py(2 hunks)backend/app/models/database/weaviate.py(2 hunks)backend/app/services/embedding_service/service.py(1 hunks)backend/config.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/agents/devrel/github/services/github_mcp_server.py (1)
backend/app/agents/devrel/github/services/github_mcp_service.py (1)
GitHubMCPService(7-103)
🪛 Ruff (0.13.3)
backend/app/agents/devrel/github/services/github_mcp_service.py
8-8: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🔇 Additional comments (3)
backend/app/agents/devrel/github/services/github_mcp_server.py (2)
17-17: Configuration centralization looks good (pending import fix).The switch from environment variables to the central config module is clean and maintains the existing behavior. The warning messages for missing values remain in place, preserving the same developer experience.
Note: This approval assumes the import path issue (line 4) is resolved.
Also applies to: 23-23
4-4: Validate config import resolution
The bareimport configwill only work ifbackend/is onPYTHONPATH(for example, by running your app from thebackend/directory). In environments where only the service package is on the path, this import fails. Ensure your entrypoint addsbackend/toPYTHONPATH, or switch to an explicit absolute import:from backend import configbackend/config.py (1)
11-12: LGTM!The GitHub configuration is clean:
- Reasonable default for GITHUB_ORG
- Good fallback from GITHUB_TOKEN to GH_TOKEN (supports GitHub CLI convention)
| import os | ||
| import json | ||
| import re | ||
| import config |
There was a problem hiding this comment.
Qualify config import through backend
Align with the package structure so the module loads reliably.
-import config
+from backend import config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import config | |
| from backend import config |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/github/github_toolkit.py around line 4, the
top-level plain "import config" should be qualified through the package path so
Python reliably resolves the module; replace the bare import with a
package-qualified import (e.g., "from backend import config" or "import
backend.config as config"), update any references to use the qualified name if
needed, and run the module/tests to confirm the import resolves under the
project package layout.
| from typing import Dict, Any, Optional, List, Union | ||
| import aiohttp | ||
| import asyncio | ||
| import config |
There was a problem hiding this comment.
Use package-qualified config import
This module runs inside the backend package. With the new config defined under backend/config.py, import config raises ModuleNotFoundError. Import it via the package-qualified path (and mirror that change in the other modules touched in this PR that pull in config).
-import config
+from backend import config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import config | |
| from backend/app/agents/devrel/github/services/github_mcp_client.py | |
| from backend import config |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/github/services/github_mcp_client.py around line 5,
the file does a top-level import "import config" which fails because config now
lives at backend/config.py; change the import to the package-qualified path
(e.g., from backend import config or import backend.config as config) and update
other modules modified in this PR that similarly use bare "import config" to use
the package-qualified import to avoid ModuleNotFoundError when running inside
the backend package.
| load_dotenv(dotenv_path=dotenv_path) | ||
| else: | ||
| load_dotenv() | ||
| import config |
There was a problem hiding this comment.
Import config from the backend package
Same module-resolution issue here—import via backend to keep the module loadable.
-import config
+from backend import config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import config | |
| from backend import config |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/github/services/github_mcp_service.py around line
5, the file currently does "import config" which fails module resolution; change
the import to use the package path (e.g., "from backend import config" or
"import backend.config as config" consistent with the project's import style)
and update any references accordingly so the module is imported via the backend
package namespace.
| @@ -1,12 +1,12 @@ | |||
| import os | |||
| import config | |||
There was a problem hiding this comment.
Import config via the backend package
For the same reason noted in the MCP client, this should pull config through backend to avoid ModuleNotFoundError.
-import config
+from backend import config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import config | |
| from backend import config |
🤖 Prompt for AI Agents
In backend/app/agents/devrel/github/tools/github_support.py at line 1, the
module currently does a top-level "import config" which can cause
ModuleNotFoundError; change the import to load config through the package
namespace (e.g., "from backend import config" or "import backend.config as
config") and update any usages to reference that imported name so the package
import resolution works correctly.
| @@ -1,21 +1,20 @@ | |||
| import logging | |||
| import os | |||
| import config | |||
There was a problem hiding this comment.
Fix config import path
config lives under backend/config.py, so this needs the package-qualified form.
-import config
+from backend import config📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import config | |
| -from backend/app/services/embedding_service/service.py | |
| from backend/app/services/embedding_service/service.py | |
| from backend import config |
🤖 Prompt for AI Agents
In backend/app/services/embedding_service/service.py around line 2, the bare
import "import config" should be changed to a package-qualified import that
points to backend/config.py; replace it with an absolute import such as "from
backend import config" (or "import backend.config as config") so the module
resolves correctly when running from the project root.
| GITHUB_TOKEN = os.getenv("GITHUB_TOKEN") or os.getenv("GH_TOKEN") | ||
|
|
||
| MODEL_NAME = os.getenv("EMBEDDING_MODEL", "BAAI/bge-small-en-v1.5") | ||
| MAX_BATCH_SIZE = int(os.getenv("EMBEDDING_MAX_BATCH_SIZE", "32")) |
There was a problem hiding this comment.
Add error handling for integer conversion.
int() will raise ValueError if EMBEDDING_MAX_BATCH_SIZE is set to a non-numeric value (e.g., "large", "auto"). This could crash the application on startup with an unclear error message.
Apply this diff to add validation:
-MAX_BATCH_SIZE = int(os.getenv("EMBEDDING_MAX_BATCH_SIZE", "32"))
+try:
+ MAX_BATCH_SIZE = int(os.getenv("EMBEDDING_MAX_BATCH_SIZE", "32"))
+except ValueError:
+ raise ValueError(
+ "EMBEDDING_MAX_BATCH_SIZE must be a valid integer. "
+ f"Got: {os.getenv('EMBEDDING_MAX_BATCH_SIZE')}"
+ )🤖 Prompt for AI Agents
In backend/config.py around line 15, the direct int(os.getenv(...)) conversion
can raise ValueError for non-numeric env values; change it to read the raw env
var, try to convert to int inside a try/except, validate that the parsed value
is a positive integer (>=1), and on failure either fall back to the default 32
and emit a clear warning log message that shows the invalid input, or raise a
new ValueError with a descriptive message — ensure you use a defined logger or
the existing logging mechanism and keep the default behavior if the env is
unset.
|
Merged! Cheers! |
2 participants
Closes #139
📝 Description
I have done the basic fixes for warnings that came in relation to depreciated pydantic models and others and grouped all the env variables from different files into a single .config file which is being used by all the files
🔧 Changes Made
Changed depreciated code in the backend to support latest version of dependency
Brought together all the environment variables under same file
📷 Screenshots or Visual Changes (if applicable)
Nil
🤝 Collaboration
nil
✅ Checklist
Summary by CodeRabbit
Refactor
Chores