fix: protect critical re-export imports from linter removal
Problem: - Ruff removed 'from app.core.database import Base' from models/database/base.py - Import appeared "unused" (F401) but was actually a critical re-export - Caused ImportError: cannot import name 'Base' at runtime - Re-export pattern: import in one file to export from package Solution: 1. Added F401 ignore for models/database/base.py in pyproject.toml 2. Created scripts/verify_critical_imports.py verification script 3. Integrated verification into make check and CI pipeline 4. Updated documentation with explanation New Verification Script: - Checks all critical re-export imports exist - Detects import variations (parentheses, 'as' clauses) - Handles SQLAlchemy declarative_base alternatives - Runs as part of make check automatically Protected Files: - models/database/base.py - Re-exports Base for all models - models/__init__.py - Exports Base for Alembic - models/database/__init__.py - Exports Base from package - All __init__.py files (already protected) Makefile Changes: - make verify-imports - Run import verification - make check - Now includes verify-imports - make ci - Includes verify-imports in pipeline Documentation Updated: - Code quality guide explains re-export protection - Pre-commit workflow includes verification - Examples of why re-exports matter This prevents future issues where linters remove seemingly "unused" imports that are actually critical for application structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
11
Makefile
11
Makefile
@@ -190,9 +190,13 @@ lint-strict:
|
||||
@echo "Type checking with mypy..."
|
||||
$(PYTHON) -m mypy .
|
||||
|
||||
check: format lint
|
||||
check: format lint verify-imports
|
||||
|
||||
ci: lint-strict test-coverage
|
||||
ci: lint-strict verify-imports test-coverage
|
||||
|
||||
verify-imports:
|
||||
@echo "Verifying critical imports..."
|
||||
$(PYTHON) scripts/verify_critical_imports.py
|
||||
|
||||
qa: format lint test-coverage docs-check
|
||||
@echo "Quality assurance checks completed!"
|
||||
@@ -333,7 +337,8 @@ help:
|
||||
@echo " format - Format code with ruff"
|
||||
@echo " lint - Lint and auto-fix with ruff + mypy"
|
||||
@echo " lint-strict - Lint without auto-fix + mypy"
|
||||
@echo " check - Format + lint"
|
||||
@echo " verify-imports - Verify critical imports haven't been removed"
|
||||
@echo " check - Format + lint + verify imports"
|
||||
@echo " ci - Full CI pipeline (strict)"
|
||||
@echo " qa - Quality assurance"
|
||||
@echo ""
|
||||
|
||||
@@ -367,9 +367,7 @@ def get_current_customer_from_cookie_or_header(
|
||||
|
||||
# Verify token hasn't expired
|
||||
exp = payload.get("exp")
|
||||
if exp and datetime.fromtimestamp(exp, tz=UTC) < datetime.now(
|
||||
UTC
|
||||
):
|
||||
if exp and datetime.fromtimestamp(exp, tz=UTC) < datetime.now(UTC):
|
||||
logger.warning(f"Expired customer token for customer_id={customer_id}")
|
||||
raise InvalidTokenException("Token has expired")
|
||||
|
||||
|
||||
@@ -48,9 +48,7 @@ class ContentPageCreate(BaseModel):
|
||||
meta_description: str | None = Field(
|
||||
None, max_length=300, description="SEO meta description"
|
||||
)
|
||||
meta_keywords: str | None = Field(
|
||||
None, max_length=300, description="SEO keywords"
|
||||
)
|
||||
meta_keywords: str | None = Field(None, max_length=300, description="SEO keywords")
|
||||
is_published: bool = Field(default=False, description="Publish immediately")
|
||||
show_in_footer: bool = Field(default=True, description="Show in footer navigation")
|
||||
show_in_header: bool = Field(default=False, description="Show in header navigation")
|
||||
|
||||
@@ -88,9 +88,7 @@ def mark_all_as_read(
|
||||
@router.get("/alerts", response_model=PlatformAlertListResponse)
|
||||
def get_platform_alerts(
|
||||
severity: str | None = Query(None, description="Filter by severity"),
|
||||
is_resolved: bool | None = Query(
|
||||
None, description="Filter by resolution status"
|
||||
),
|
||||
is_resolved: bool | None = Query(None, description="Filter by resolution status"),
|
||||
skip: int = Query(0, ge=0),
|
||||
limit: int = Query(50, ge=1, le=100),
|
||||
db: Session = Depends(get_db),
|
||||
|
||||
@@ -30,9 +30,7 @@ def get_product_catalog(
|
||||
skip: int = Query(0, ge=0),
|
||||
limit: int = Query(100, ge=1, le=1000),
|
||||
search: str | None = Query(None, description="Search products by name"),
|
||||
is_featured: bool | None = Query(
|
||||
None, description="Filter by featured products"
|
||||
),
|
||||
is_featured: bool | None = Query(None, description="Filter by featured products"),
|
||||
db: Session = Depends(get_db),
|
||||
):
|
||||
"""
|
||||
|
||||
4
app/api/v1/vendor/content_pages.py
vendored
4
app/api/v1/vendor/content_pages.py
vendored
@@ -43,9 +43,7 @@ class VendorContentPageCreate(BaseModel):
|
||||
meta_description: str | None = Field(
|
||||
None, max_length=300, description="SEO meta description"
|
||||
)
|
||||
meta_keywords: str | None = Field(
|
||||
None, max_length=300, description="SEO keywords"
|
||||
)
|
||||
meta_keywords: str | None = Field(None, max_length=300, description="SEO keywords")
|
||||
is_published: bool = Field(default=False, description="Publish immediately")
|
||||
show_in_footer: bool = Field(default=True, description="Show in footer navigation")
|
||||
show_in_header: bool = Field(default=False, description="Show in header navigation")
|
||||
|
||||
@@ -13,7 +13,6 @@ Note: Environment detection is handled by app.core.environment module.
|
||||
This module focuses purely on configuration storage and validation.
|
||||
"""
|
||||
|
||||
|
||||
from pydantic_settings import BaseSettings
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Authentication and authorization specific exceptions.
|
||||
"""
|
||||
|
||||
|
||||
from .base import AuthenticationException, AuthorizationException, ConflictException
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Shopping cart specific exceptions.
|
||||
"""
|
||||
|
||||
|
||||
from .base import BusinessLogicException, ResourceNotFoundException, ValidationException
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Order management specific exceptions.
|
||||
"""
|
||||
|
||||
|
||||
from .base import BusinessLogicException, ResourceNotFoundException, ValidationException
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Product (vendor catalog) specific exceptions.
|
||||
"""
|
||||
|
||||
|
||||
from .base import (
|
||||
BusinessLogicException,
|
||||
ConflictException,
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Vendor domain management specific exceptions.
|
||||
"""
|
||||
|
||||
|
||||
from .base import (
|
||||
BusinessLogicException,
|
||||
ConflictException,
|
||||
|
||||
@@ -30,7 +30,6 @@ Routes:
|
||||
- GET /code-quality/violations/{violation_id} → Violation details (auth required)
|
||||
"""
|
||||
|
||||
|
||||
from fastapi import APIRouter, Depends, Path, Request
|
||||
from fastapi.responses import HTMLResponse, RedirectResponse
|
||||
from fastapi.templating import Jinja2Templates
|
||||
|
||||
@@ -107,9 +107,7 @@ class ProductService:
|
||||
)
|
||||
|
||||
if existing:
|
||||
raise ProductAlreadyExistsException(
|
||||
"Product already exists in catalog"
|
||||
)
|
||||
raise ProductAlreadyExistsException("Product already exists in catalog")
|
||||
|
||||
# Create product
|
||||
product = Product(
|
||||
|
||||
@@ -108,9 +108,7 @@ class PriceProcessor:
|
||||
r"([A-Z]{3})\s*([0-9.,]+)": lambda m: (m.group(2), m.group(1)),
|
||||
}
|
||||
|
||||
def parse_price_currency(
|
||||
self, price_str: any
|
||||
) -> tuple[str | None, str | None]:
|
||||
def parse_price_currency(self, price_str: any) -> tuple[str | None, str | None]:
|
||||
"""
|
||||
Parse a price string to extract the numeric value and currency.
|
||||
|
||||
|
||||
@@ -190,7 +190,7 @@ show_missing = true
|
||||
Before committing code:
|
||||
|
||||
```bash
|
||||
# 1. Format and lint your code
|
||||
# 1. Format, lint, and verify critical imports
|
||||
make check
|
||||
|
||||
# 2. Run relevant tests
|
||||
@@ -201,6 +201,25 @@ git add .
|
||||
git commit -m "your message"
|
||||
```
|
||||
|
||||
### Critical Import Verification
|
||||
|
||||
The `make check` command includes a critical import verification step that ensures re-export imports haven't been removed by linters.
|
||||
|
||||
**What it checks:**
|
||||
- `models/database/base.py` - Re-exports Base from app.core.database
|
||||
- `models/__init__.py` - Exports Base for Alembic
|
||||
- `models/database/__init__.py` - Exports Base from database package
|
||||
|
||||
**Why it matters:**
|
||||
Linters like Ruff may see these imports as "unused" (F401) because they're re-exported, not directly used. Removing them breaks the application.
|
||||
|
||||
**Manual verification:**
|
||||
```bash
|
||||
make verify-imports
|
||||
```
|
||||
|
||||
If this fails, imports have been removed and must be restored.
|
||||
|
||||
## CI/CD Integration
|
||||
|
||||
For continuous integration:
|
||||
|
||||
@@ -204,9 +204,7 @@ class AuthManager:
|
||||
raise InvalidTokenException("Token missing expiration")
|
||||
|
||||
# Check if token has expired (additional check beyond jwt.decode)
|
||||
if datetime.now(UTC) > datetime.fromtimestamp(
|
||||
exp, tz=UTC
|
||||
):
|
||||
if datetime.now(UTC) > datetime.fromtimestamp(exp, tz=UTC):
|
||||
raise TokenExpiredException()
|
||||
|
||||
# Validate user identifier claim exists
|
||||
|
||||
@@ -141,9 +141,7 @@ class VendorContextManager:
|
||||
f"[OK] Vendor found via custom domain: {domain} → {vendor.name}"
|
||||
)
|
||||
return vendor
|
||||
logger.warning(
|
||||
f"No active vendor found for custom domain: {domain}"
|
||||
)
|
||||
logger.warning(f"No active vendor found for custom domain: {domain}")
|
||||
return None
|
||||
|
||||
# Method 2 & 3: Subdomain or path-based lookup
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
# models/database/cart.py
|
||||
"""Cart item database model."""
|
||||
|
||||
|
||||
from sqlalchemy import (
|
||||
Column,
|
||||
Float,
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
|
||||
from sqlalchemy import (
|
||||
JSON,
|
||||
Boolean,
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
|
||||
from sqlalchemy import Column, DateTime, ForeignKey, Index, Integer, String, Text
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
|
||||
from sqlalchemy import (
|
||||
Column,
|
||||
Index,
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Vendor Domain Model - Maps custom domains to vendors
|
||||
"""
|
||||
|
||||
|
||||
from sqlalchemy import (
|
||||
Boolean,
|
||||
Column,
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Pydantic schemas for shopping cart operations.
|
||||
"""
|
||||
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
|
||||
# ============================================================================
|
||||
|
||||
@@ -11,9 +11,7 @@ class ProductCreate(BaseModel):
|
||||
marketplace_product_id: int = Field(
|
||||
..., description="MarketplaceProduct ID to add to vendor catalog"
|
||||
)
|
||||
product_id: str | None = Field(
|
||||
None, description="Vendor's internal SKU/product ID"
|
||||
)
|
||||
product_id: str | None = Field(None, description="Vendor's internal SKU/product ID")
|
||||
price: float | None = Field(None, ge=0)
|
||||
sale_price: float | None = Field(None, ge=0)
|
||||
currency: str | None = None
|
||||
|
||||
@@ -31,7 +31,6 @@ class RoleCreate(RoleBase):
|
||||
"""Schema for creating a role."""
|
||||
|
||||
|
||||
|
||||
class RoleUpdate(BaseModel):
|
||||
"""Schema for updating a role."""
|
||||
|
||||
|
||||
@@ -100,9 +100,7 @@ class VendorUpdate(BaseModel):
|
||||
subdomain: str | None = Field(None, min_length=2, max_length=100)
|
||||
|
||||
# Business Contact Information (Vendor Fields)
|
||||
contact_email: str | None = Field(
|
||||
None, description="Public business contact email"
|
||||
)
|
||||
contact_email: str | None = Field(None, description="Public business contact email")
|
||||
contact_phone: str | None = None
|
||||
website: str | None = None
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
Pydantic schemas for vendor theme operations.
|
||||
"""
|
||||
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
|
||||
@@ -54,14 +53,10 @@ class VendorThemeUpdate(BaseModel):
|
||||
theme_name: str | None = Field(None, description="Theme preset name")
|
||||
colors: dict[str, str] | None = Field(None, description="Color scheme")
|
||||
fonts: dict[str, str] | None = Field(None, description="Font settings")
|
||||
branding: dict[str, str | None] | None = Field(
|
||||
None, description="Branding assets"
|
||||
)
|
||||
branding: dict[str, str | None] | None = Field(None, description="Branding assets")
|
||||
layout: dict[str, str] | None = Field(None, description="Layout settings")
|
||||
custom_css: str | None = Field(None, description="Custom CSS rules")
|
||||
social_links: dict[str, str] | None = Field(
|
||||
None, description="Social media links"
|
||||
)
|
||||
social_links: dict[str, str] | None = Field(None, description="Social media links")
|
||||
|
||||
|
||||
class VendorThemeResponse(BaseModel):
|
||||
|
||||
@@ -60,8 +60,10 @@ unfixable = []
|
||||
|
||||
# Per-file ignores
|
||||
[tool.ruff.lint.per-file-ignores]
|
||||
# Ignore import violations in __init__.py files
|
||||
# Ignore import violations in __init__.py files (re-exports)
|
||||
"__init__.py" = ["F401", "F403"]
|
||||
# Base files that re-export (re-export Base from core.database)
|
||||
"models/database/base.py" = ["F401"]
|
||||
# Ignore specific rules in test files
|
||||
"tests/**/*.py" = ["S101", "PLR2004"]
|
||||
# Alembic migrations can have longer lines and specific patterns
|
||||
|
||||
@@ -7,7 +7,6 @@ Usage: python route_diagnostics.py
|
||||
"""
|
||||
|
||||
|
||||
|
||||
def check_route_order():
|
||||
"""Check if routes are registered in the correct order."""
|
||||
print("🔍 Checking FastAPI Route Configuration...\n")
|
||||
|
||||
@@ -20,7 +20,6 @@ Requirements:
|
||||
* Customer: username=customer, password=customer123, vendor_id=1
|
||||
"""
|
||||
|
||||
|
||||
import requests
|
||||
|
||||
BASE_URL = "http://localhost:8000"
|
||||
|
||||
@@ -9,7 +9,6 @@ Tests:
|
||||
- Transfer ownership
|
||||
"""
|
||||
|
||||
|
||||
import requests
|
||||
|
||||
BASE_URL = "http://localhost:8000"
|
||||
|
||||
156
scripts/verify_critical_imports.py
Executable file
156
scripts/verify_critical_imports.py
Executable file
@@ -0,0 +1,156 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Verify Critical Imports
|
||||
========================
|
||||
Checks that critical imports (re-exports) haven't been removed by linters.
|
||||
|
||||
This script verifies that essential import statements exist in key files,
|
||||
preventing issues where tools like Ruff might remove imports that appear
|
||||
unused but are actually critical for the application structure.
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
# Define critical imports that must exist
|
||||
# Format: {file_path: [(import_line, description)]}
|
||||
CRITICAL_IMPORTS: dict[str, list[tuple[str, str]]] = {
|
||||
"models/database/base.py": [
|
||||
("from app.core.database import Base", "Re-export Base for all models"),
|
||||
],
|
||||
"models/__init__.py": [
|
||||
("from .database.base import Base", "Export Base for Alembic and models"),
|
||||
],
|
||||
"models/database/__init__.py": [
|
||||
("from .base import Base", "Export Base from database package"),
|
||||
],
|
||||
"app/core/database.py": [
|
||||
(
|
||||
"from sqlalchemy.ext.declarative import declarative_base",
|
||||
"SQLAlchemy Base declaration",
|
||||
),
|
||||
# Note: Might also use sqlalchemy.orm declarative_base in newer versions
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
class ImportVerifier:
|
||||
"""Verifies critical imports exist in codebase"""
|
||||
|
||||
def __init__(self, project_root: Path):
|
||||
self.project_root = project_root
|
||||
self.issues: list[str] = []
|
||||
|
||||
def verify_all(self) -> bool:
|
||||
"""Verify all critical imports"""
|
||||
print("🔍 Verifying critical imports...\n")
|
||||
|
||||
all_good = True
|
||||
for file_path, imports in CRITICAL_IMPORTS.items():
|
||||
if not self.verify_file(file_path, imports):
|
||||
all_good = False
|
||||
|
||||
return all_good
|
||||
|
||||
def verify_file(
|
||||
self, file_path: str, required_imports: list[tuple[str, str]]
|
||||
) -> bool:
|
||||
"""Verify imports in a single file"""
|
||||
full_path = self.project_root / file_path
|
||||
|
||||
if not full_path.exists():
|
||||
self.issues.append(f"❌ File not found: {file_path}")
|
||||
print(f"❌ {file_path}: File not found")
|
||||
return False
|
||||
|
||||
content = full_path.read_text()
|
||||
file_ok = True
|
||||
|
||||
for import_line, description in required_imports:
|
||||
# Check for exact import or variations
|
||||
if import_line in content:
|
||||
print(f"✅ {file_path}: {import_line}")
|
||||
else:
|
||||
# Check for alternative import formats
|
||||
alternatives = self._get_import_alternatives(import_line)
|
||||
found = any(alt in content for alt in alternatives)
|
||||
|
||||
if found:
|
||||
print(f"✅ {file_path}: {import_line} (alternative format)")
|
||||
else:
|
||||
self.issues.append(
|
||||
f"❌ {file_path}: Missing critical import\n"
|
||||
f" Expected: {import_line}\n"
|
||||
f" Purpose: {description}"
|
||||
)
|
||||
print(f"❌ {file_path}: Missing {import_line}")
|
||||
file_ok = False
|
||||
|
||||
print()
|
||||
return file_ok
|
||||
|
||||
def _get_import_alternatives(self, import_line: str) -> list[str]:
|
||||
"""Get alternative formats for an import"""
|
||||
alternatives = [import_line]
|
||||
|
||||
# Handle 'from x import y' vs 'from x import (y)'
|
||||
if "from" in import_line and "import" in import_line:
|
||||
parts = import_line.split("import")
|
||||
if len(parts) == 2:
|
||||
from_part = parts[0].strip()
|
||||
import_part = parts[1].strip()
|
||||
|
||||
# Add parenthesized version
|
||||
alternatives.append(f"{from_part} import ({import_part})")
|
||||
|
||||
# Add version with 'as' clause
|
||||
alternatives.append(f"{import_line} as")
|
||||
|
||||
# Handle declarative_base alternatives (sqlalchemy changes)
|
||||
if "declarative_base" in import_line:
|
||||
# Old style
|
||||
alternatives.append(
|
||||
"from sqlalchemy.ext.declarative import declarative_base"
|
||||
)
|
||||
# New style (SQLAlchemy 1.4+)
|
||||
alternatives.append("from sqlalchemy.orm import declarative_base")
|
||||
|
||||
return alternatives
|
||||
|
||||
def print_summary(self):
|
||||
"""Print summary of verification"""
|
||||
print("\n" + "=" * 80)
|
||||
print("📊 CRITICAL IMPORTS VERIFICATION SUMMARY")
|
||||
print("=" * 80)
|
||||
|
||||
if not self.issues:
|
||||
print("\n✅ All critical imports verified successfully!")
|
||||
print("\nAll re-export patterns are intact.")
|
||||
else:
|
||||
print(f"\n❌ Found {len(self.issues)} issue(s):\n")
|
||||
for issue in self.issues:
|
||||
print(issue)
|
||||
print()
|
||||
|
||||
print("💡 RESOLUTION:")
|
||||
print(" 1. Check if imports were removed by linter (Ruff)")
|
||||
print(" 2. Add missing imports back to the files")
|
||||
print(" 3. Update pyproject.toml to ignore F401 for these files")
|
||||
print(" 4. Run this script again to verify")
|
||||
|
||||
print("=" * 80)
|
||||
|
||||
|
||||
def main():
|
||||
"""Main entry point"""
|
||||
project_root = Path(__file__).parent.parent
|
||||
|
||||
verifier = ImportVerifier(project_root)
|
||||
success = verifier.verify_all()
|
||||
verifier.print_summary()
|
||||
|
||||
sys.exit(0 if success else 1)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -6,7 +6,6 @@ Tests the complete error handling flow from FastAPI through custom exception han
|
||||
to ensure proper HTTP status codes, error structures, and client-friendly responses.
|
||||
"""
|
||||
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ Tests cover:
|
||||
- Edge cases and isolation
|
||||
"""
|
||||
|
||||
|
||||
import pytest
|
||||
|
||||
from app.exceptions.base import RateLimitException
|
||||
|
||||
Reference in New Issue
Block a user