-
Notifications
You must be signed in to change notification settings - Fork 0
Description
What happened?
load_env(): Reduced from 3 calls to 1.- Redirect Route: Consolidate into
main.pyonly. - Duplicate logic in
create_short_url: Flattened theif/elsetree so the session is updated only once at the end. - DB Re-connects: Removed the manual
try: connect_db()checks inside every route; thelifespanensures it's handled at boot.
What did you expect to happen?
You have some redundant logic and "racing" configurations that can be simplified. The biggest issue is that you are calling load_env() in every single file and manually managing database connections in multiple places.
Here are the specific redundant and unnecessary parts of your code:
1. Redundant load_env() Calls
You are calling load_env() in app/db/data.py, app/main.py, and app/api/fast_api.py.
- The Fix: Call it once at the very top of your entry point (
app/main.py). Since the other modules are imported bymain.py, the environment variables will already be available to them.
2. Duplicate Redirection Logic
You have a /{short_code} redirect route in both app/main.py and app/api/fast_api.py.
- The Redundancy: Since you are mounting the API app inside the main app (
app.mount("/api", api_app)), having the same route in both places causes confusion and code duplication. - The Fix: Keep the redirection in the main app (
app/main.py) for the frontend users, and remove it from the API file unless you want the API to specifically handle its own redirects under/api/v1/{short_code}.
3. "Connection on Import" Pattern
In app/db/data.py, you call connect_db() at the bottom of the file.
- The Risk: This runs as soon as any file imports
data.py. If the database is slow to respond, it can hang your app startup or cause issues during testing. - The Fix: Use FastAPI's Lifespan events to connect to the database once when the app starts.
4. Code Duplication in create_short_url
In app/main.py, inside create_short_url, you have the exact same logic (generating a code and updating the session) repeated three times: once for no-db mode, once if find_one fails, and once at the end.
Optimized Code Structure
1. Simplified app/db/data.py
Remove the connect_db() call from the bottom and handle the pymongo import safely.
import os
from typing import Any
# SAFE IMPORT: App won't crash if pymongo is missing
try:
from pymongo import MongoClient
from pymongo.errors import ServerSelectionTimeoutError
MONGO_INSTALLED = True
except ImportError:
MONGO_INSTALLED = False
client: Any = None
db: Any = None
urls: Any = None
url_stats: Any = None
def connect_db():
global client, db, urls, url_stats
if not MONGO_INSTALLED:
print("⚠️ pymongo is not installed. Running in NO-DB mode.")
return False
MONGO_URI = os.getenv("MONGO_URI")
DB_NAME = os.getenv("DATABASE_NAME", "tiny_url")
if not MONGO_URI:
return False
try:
client = MongoClient(MONGO_URI, serverSelectionTimeoutMS=2000)
client.admin.command("ping")
db = client[DB_NAME]
urls = db["urls"]
url_stats = db["url_stats"]
return True
except Exception:
return False2. Cleaned up app/main.py
Use the lifespan to connect once and clean up the redundancy.
from contextlib import asynccontextmanager
from app.db.data import connect_db, urls
@asynccontextmanager
async def lifespan(app: FastAPI):
# Load env once at the start
load_env()
# Connect once
connect_db()
yield
app = FastAPI(title="TinyURL", lifespan=lifespan)
# Simplify your creation logic to avoid repeating session.update()
@app.post("/", response_class=RedirectResponse)
async def create_short_url(request: Request, original_url: str = Form(""), ...):
# 1. Validation Logic...
# 2. Logic to get short_code
short_code = None
if urls is not None:
try:
existing = urls.find_one({"original_url": original_url})
short_code = existing["short_code"] if existing else None
except Exception:
pass
if not short_code:
short_code = generate_code()
# Save to DB if possible
if urls is not None:
try:
urls.insert_one({"short_code": short_code, ...})
except Exception:
pass
# 3. One single session update at the end
request.session.update({"short_code": short_code, ...})
return RedirectResponse("/", status_code=303)Anything else we need to know?
No response
What browsers are you seeing the problem on?
No response
Relevant log output
Contact Details
No response
Code of Conduct
- I agree to follow this project's Code of Conduct