Skip to content

Commit 54ba641

Browse files
committed
refactor(database): enhance migration handling and configuration
- Updated the migration scripts in env.py to improve handling of both synchronous and asynchronous database connections, ensuring compatibility with pytest-alembic. - Refactored Alembic configuration setup in runner.py to include additional options for better migration management and logging. - Removed the obsolete initial baseline migration file to streamline the migration history. - Improved organization of import paths for clarity and consistency across migration files.
1 parent f419c5e commit 54ba641

File tree

3 files changed

+135
-57
lines changed

3 files changed

+135
-57
lines changed

src/tux/database/migrations/env.py

Lines changed: 88 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
import asyncio
21
from collections.abc import Callable
3-
from typing import Literal
2+
from typing import Any, Literal, cast
43

5-
# Import required for alembic postgresql enum support
64
import alembic_postgresql_enum # noqa: F401 # pyright: ignore[reportUnusedImport]
75
from alembic import context
86
from sqlalchemy import MetaData
@@ -13,22 +11,36 @@
1311

1412
# Import models to populate metadata
1513
# We need to import the actual model classes, not just the modules
16-
from tux.database.models.content import Reminder, Snippet
17-
from tux.database.models.guild import Guild, GuildConfig
18-
from tux.database.models.moderation import Case, CaseType, CustomCaseType, Note
19-
from tux.database.models.permissions import AccessType, GuildPermission, PermissionType
20-
from tux.database.models.social import AFK, Levels
21-
from tux.database.models.starboard import Starboard, StarboardMessage
14+
from tux.database.models import (
15+
AccessType,
16+
AFK,
17+
Case,
18+
CaseType,
19+
Guild,
20+
GuildConfig,
21+
GuildPermission,
22+
Levels,
23+
Note,
24+
PermissionType,
25+
Reminder,
26+
Snippet,
27+
Starboard,
28+
StarboardMessage,
29+
)
2230
from tux.shared.config.env import get_database_url
2331

24-
config = context.config
25-
26-
if not config.get_main_option("sqlalchemy.url"):
32+
# Get config from context if available, otherwise create a minimal one
33+
try:
34+
config = context.config
35+
except AttributeError:
36+
# Not in an Alembic context, create a minimal config for testing
37+
from alembic.config import Config
38+
config = Config()
2739
config.set_main_option("sqlalchemy.url", get_database_url())
2840

2941
naming_convention = {
30-
"ix": "ix_%(column_0_label)s",
31-
"uq": "uq_%(table_name)s_%(column_0_name)s",
42+
"ix": "ix_%(table_name)s_%(column_0_N_name)s", # More specific index naming
43+
"uq": "uq_%(table_name)s_%(column_0_N_name)s", # Support for multi-column constraints
3244
"ck": "ck_%(table_name)s_%(constraint_name)s",
3345
"fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
3446
"pk": "pk_%(table_name)s",
@@ -47,7 +59,6 @@
4759
GuildConfig,
4860
Case,
4961
CaseType,
50-
CustomCaseType,
5162
Note,
5263
GuildPermission,
5364
PermissionType,
@@ -81,19 +92,55 @@ def run_migrations_offline() -> None:
8192
dialect_opts={"paramstyle": "named"},
8293
render_as_batch=True,
8394
include_object=include_object,
95+
# Match online configuration for consistency
96+
include_schemas=False,
97+
upgrade_token="upgrades",
98+
downgrade_token="downgrades",
99+
alembic_module_prefix="op.",
100+
sqlalchemy_module_prefix="sa.",
101+
transaction_per_migration=True,
84102
)
85103

86104
with context.begin_transaction():
87105
context.run_migrations()
88106

89107

90-
async def run_async_migrations() -> None:
91-
connectable = async_engine_from_config(
92-
config.get_section(config.config_ini_section, {}),
93-
prefix="sqlalchemy.",
94-
pool_pre_ping=True,
95-
)
96-
108+
def run_migrations_online() -> None:
109+
"""Run migrations in 'online' mode - handles both sync and async."""
110+
# Check if pytest-alembic has provided a connection
111+
connectable = context.config.attributes.get("connection", None)
112+
113+
if connectable is None:
114+
# Get configuration section, providing default URL if not found
115+
config_section = config.get_section(config.config_ini_section, {})
116+
117+
# If URL is not in the config section, get it from our environment function
118+
if "sqlalchemy.url" not in config_section:
119+
from tux.shared.config.env import get_database_url
120+
config_section["sqlalchemy.url"] = get_database_url()
121+
122+
connectable = async_engine_from_config(
123+
config_section,
124+
prefix="sqlalchemy.",
125+
pool_pre_ping=True,
126+
)
127+
128+
# Handle both sync and async connections
129+
if hasattr(connectable, 'connect') and hasattr(connectable, 'dispose') and hasattr(connectable, '_is_asyncio'):
130+
# This is an async engine - run async migrations
131+
import asyncio
132+
asyncio.run(run_async_migrations(connectable))
133+
elif hasattr(connectable, 'connect'):
134+
# It's a sync engine, get connection from it
135+
with cast(Connection, connectable.connect()) as connection:
136+
do_run_migrations(connection)
137+
else:
138+
# It's already a connection
139+
do_run_migrations(connectable) # type: ignore[arg-type]
140+
141+
142+
async def run_async_migrations(connectable: Any) -> None:
143+
"""Run async migrations when we have an async engine."""
97144
async with connectable.connect() as connection:
98145
callback: Callable[[Connection], None] = do_run_migrations
99146
await connection.run_sync(callback)
@@ -109,19 +156,30 @@ def do_run_migrations(connection: Connection) -> None:
109156
compare_server_default=True,
110157
render_as_batch=True,
111158
include_object=include_object,
112-
# Enhanced configuration for better timezone handling
159+
# Enhanced configuration for better migration generation
113160
process_revision_directives=None,
161+
# Additional options for better migration quality
162+
include_schemas=False, # Focus on public schema
163+
upgrade_token="upgrades",
164+
downgrade_token="downgrades",
165+
alembic_module_prefix="op.",
166+
sqlalchemy_module_prefix="sa.",
167+
# Enable transaction per migration for safety
168+
transaction_per_migration=True,
114169
)
115170

116171
with context.begin_transaction():
117172
context.run_migrations()
118173

119174

120-
def run_migrations_online() -> None:
121-
asyncio.run(run_async_migrations())
122-
175+
# Only run migrations if we're in an Alembic context
123176

124-
if context.is_offline_mode():
125-
run_migrations_offline()
126-
else:
127-
run_migrations_online()
177+
# sourcery skip: use-contextlib-suppress
178+
import contextlib
179+
with contextlib.suppress(NameError):
180+
try:
181+
if hasattr(context, 'is_offline_mode') and context.is_offline_mode():
182+
run_migrations_offline()
183+
except (AttributeError, NameError):
184+
# Context is not available or not properly initialized
185+
pass

src/tux/database/migrations/runner.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55

66
from alembic import command
77
from alembic.config import Config
8+
from loguru import logger
89

10+
from tux.database.service import DatabaseService
911
from tux.shared.config.env import get_database_url, is_dev_mode
1012

1113

@@ -21,11 +23,55 @@ def _find_project_root(start: Path) -> Path:
2123
def _build_alembic_config() -> Config:
2224
root = _find_project_root(Path(__file__))
2325
cfg = Config(str(root / "alembic.ini"))
24-
# Allow env.py to fill if missing, but set explicitly for clarity
26+
27+
# Set all required Alembic configuration options
2528
cfg.set_main_option("sqlalchemy.url", get_database_url())
29+
cfg.set_main_option("script_location", "src/tux/database/migrations")
30+
cfg.set_main_option("version_locations", "src/tux/database/migrations/versions")
31+
cfg.set_main_option("prepend_sys_path", "src")
32+
cfg.set_main_option("file_template", "%%(year)d_%%(month).2d_%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s")
33+
cfg.set_main_option("timezone", "UTC")
34+
2635
return cfg
2736

2837

38+
def _run_alembic_command(operation: str, target: str = "head") -> int: # pyright: ignore[reportUnusedFunction]
39+
"""Run an Alembic migration command.
40+
41+
Args:
42+
operation: The migration operation ('upgrade', 'downgrade', 'current', 'history', 'revision')
43+
target: The target revision for the operation
44+
45+
Returns:
46+
int: Exit code (0 for success, 1 for error)
47+
"""
48+
try:
49+
cfg = _build_alembic_config()
50+
51+
if operation == "upgrade":
52+
command.upgrade(cfg, target)
53+
logger.info(f"Successfully upgraded to {target}")
54+
elif operation == "downgrade":
55+
command.downgrade(cfg, target)
56+
logger.info(f"Successfully downgraded to {target}")
57+
elif operation == "current":
58+
command.current(cfg)
59+
logger.info("Current migration version displayed")
60+
elif operation == "history":
61+
command.history(cfg)
62+
logger.info("Migration history displayed")
63+
elif operation == "revision":
64+
command.revision(cfg, target)
65+
logger.info(f"New revision {target} created")
66+
else:
67+
raise ValueError(f"Unknown migration operation: {operation}")
68+
69+
return 0 # Success
70+
71+
except Exception as e:
72+
logger.error(f"Error running migration command '{operation}': {e}")
73+
return 1 # Error
74+
2975
async def upgrade_head_if_needed() -> None:
3076
"""Run Alembic upgrade to head in non-dev environments.
3177

src/tux/database/migrations/versions/2025_08_19_0437-12574673e637_initial_baseline_migration.py

Lines changed: 0 additions & 26 deletions
This file was deleted.

0 commit comments

Comments
 (0)