config: read Google credentials/project id from environment (small PR)#2
config: read Google credentials/project id from environment (small PR)#2AsadUllah428 wants to merge 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughA configuration file was updated to use environment variables instead of hardcoded credentials. The changes add an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 1
🧹 Nitpick comments (1)
config.py (1)
13-13: Variable name collides with one of the env-var names it reads.
GOOGLE_CREDENTIALS_PATHis both the Python constant and the second env-var key being queried. This works but can confuse readers (and tools likegrep) about whether a reference is to the Python symbol or the env var. Consider renaming the fallback env var lookup to something distinct, or document the precedence clearly in the comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config.py` at line 13, The constant GOOGLE_CREDENTIALS_PATH currently re-uses the same name as the second environment variable lookup which is confusing; change the second env-var key to a distinct name (e.g. "GOOGLE_CREDENTIALS_FILE" or "GOOGLE_CREDENTIALS_PATH_FALLBACK") and update the assignment in the GOOGLE_CREDENTIALS_PATH initializer to read os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") or os.environ.get("<NEW_ENV_VAR_NAME>") or "your path here", and add a brief comment above explaining the precedence order (APPLICATION_CREDENTIALS first, fallback env var second, then default string).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config.py`:
- Around line 13-14: Call dotenv.load_dotenv() at the top of the module to pick
up .env files, stop using placeholder strings as defaults for
GOOGLE_CREDENTIALS_PATH and PROJECT_ID, and instead read them via os.environ.get
into variables and validate immediately: if GOOGLE_CREDENTIALS_PATH or
PROJECT_ID is missing/empty raise a clear RuntimeError (or log.error then raise)
with a message that names the missing env var(s) and guidance (e.g. set
GOOGLE_APPLICATION_CREDENTIALS or GOOGLE_CREDENTIALS_PATH, set PROJECT_ID) so
failures fail fast; update references to the symbols GOOGLE_CREDENTIALS_PATH and
PROJECT_ID accordingly.
---
Nitpick comments:
In `@config.py`:
- Line 13: The constant GOOGLE_CREDENTIALS_PATH currently re-uses the same name
as the second environment variable lookup which is confusing; change the second
env-var key to a distinct name (e.g. "GOOGLE_CREDENTIALS_FILE" or
"GOOGLE_CREDENTIALS_PATH_FALLBACK") and update the assignment in the
GOOGLE_CREDENTIALS_PATH initializer to read
os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") or
os.environ.get("<NEW_ENV_VAR_NAME>") or "your path here", and add a brief
comment above explaining the precedence order (APPLICATION_CREDENTIALS first,
fallback env var second, then default string).
| GOOGLE_CREDENTIALS_PATH = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") or os.environ.get("GOOGLE_CREDENTIALS_PATH") or "your path here" | ||
| PROJECT_ID = os.environ.get("PROJECT_ID") or "your id here" |
There was a problem hiding this comment.
Placeholder fallback defaults will cause cryptic failures at runtime.
If neither environment variable is set, GOOGLE_CREDENTIALS_PATH silently becomes "your path here" and PROJECT_ID becomes "your id here". Downstream code will then fail with an opaque file-not-found or authentication error instead of a clear configuration message. Prefer raising early or at least logging a warning.
Also, python-dotenv is listed as a project dependency — consider calling dotenv.load_dotenv() at the top of this module so .env files are picked up automatically.
🛡️ Proposed fix: fail fast on missing config
import os
+from dotenv import load_dotenv
+
+load_dotenv()
CSV_FILENAME = "leads.csv"
APP_NAME = "sales_agent_app"
@@
# Google Service Account (path or env)
-GOOGLE_CREDENTIALS_PATH = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") or os.environ.get("GOOGLE_CREDENTIALS_PATH") or "your path here"
-PROJECT_ID = os.environ.get("PROJECT_ID") or "your id here"
+GOOGLE_CREDENTIALS_PATH = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS") or os.environ.get("GOOGLE_CREDENTIALS_PATH")
+if not GOOGLE_CREDENTIALS_PATH:
+ raise RuntimeError("Set GOOGLE_APPLICATION_CREDENTIALS or GOOGLE_CREDENTIALS_PATH in the environment")
+
+PROJECT_ID = os.environ.get("PROJECT_ID")
+if not PROJECT_ID:
+ raise RuntimeError("Set PROJECT_ID in the environment")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config.py` around lines 13 - 14, Call dotenv.load_dotenv() at the top of the
module to pick up .env files, stop using placeholder strings as defaults for
GOOGLE_CREDENTIALS_PATH and PROJECT_ID, and instead read them via os.environ.get
into variables and validate immediately: if GOOGLE_CREDENTIALS_PATH or
PROJECT_ID is missing/empty raise a clear RuntimeError (or log.error then raise)
with a message that names the missing env var(s) and guidance (e.g. set
GOOGLE_APPLICATION_CREDENTIALS or GOOGLE_CREDENTIALS_PATH, set PROJECT_ID) so
failures fail fast; update references to the symbols GOOGLE_CREDENTIALS_PATH and
PROJECT_ID accordingly.
1 participant
@
Summary by CodeRabbit