Skip to content

Remove rich library from fastmigrate #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
algal merged 5 commits into from
Jun 9, 2025
Merged

Remove rich library from fastmigrate #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ed21ff3
Remove rich library from fastmigrate
pydanny May 13, 2025
a42505d
Removes reporting of migration timing.
algal Jun 9, 2025
5edea47
refactor stderr.write to print(...,file=stderr)
algal Jun 9, 2025
70bc0d5
Streamline error message and link to instructions
algal Jun 9, 2025
3822a48
Report migration failures only via stderr
algal Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 37 additions & 53 deletions fastmigrate/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,12 @@
import sqlite3
import subprocess
import sys
import time
from sys import stderr
import warnings
from datetime import datetime
from pathlib import Path
from typing import Dict, Optional

from rich.console import Console

# Initialize Rich console
console = Console()

__all__ = ["run_migrations", "create_db", "get_db_version", "create_db_backup",
# deprecated
Expand Down Expand Up @@ -242,14 +238,14 @@ def execute_sql_script(db_path: Path, script_path: Path) -> bool:

except sqlite3.Error as e:
# SQL error occurred
console.print(f"[bold red]Error[/bold red] executing SQL script {script_path}:")
console.print(f" {e}", style="red")
print(f"Error executing SQL script {script_path}:", file=stderr)
print(f" {e}", file=stderr)
return False

except Exception as e:
# Handle other errors (file not found, etc.)
console.print(f"[bold red]Error[/bold red] executing SQL script {script_path}:")
console.print(f" {e}", style="red")
print(f"Error executing SQL script {script_path}:", file=stderr)
print(f" {e}", file=stderr)
return False

finally:
Expand All @@ -269,8 +265,9 @@ def execute_python_script(db_path: Path, script_path: Path) -> bool:
)
return True
except subprocess.CalledProcessError as e:
console.print(f"[bold red]Error[/bold red] executing Python script {script_path}:")
console.print(e.stderr.decode(), style="red")
print(f"Error executing Python script {script_path}:", file=stderr)
sys.stderr.write(e.stderr.decode())
Copy link
Contributor

@jph00 jph00 Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my curiosity - why not use print here?

Copy link
Contributor

@algal algal Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My very brief thinking here was that since the string being printed was not UI copy defined in the code, but rather was a value being provided by other bits of API, then sys.stderr.write communicated more clearly to the reader that this code is simply conveying the value directly without any modifications, such as the newline automatically added by print.

Of course I could suppress that and do print(e.stderr.decode(),end='',file=stderr). This has the pro of being more consistent with the other uses of print, but the con of being a little more roundabout.

So this seemed a little cleaner? But if I'd put more sugar in my coffee in the morning, I probably would have gone the other way. Borderline case!

Copy link
Contributor

@jph00 jph00 Jun 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you then immediately print a newline on the next line, so you end up with two lines instead of one IIUC…

FWIW, for this reader at least, sys.stderr.write communicated to me that you're doing something special/different, and I should try to figure out why! :D

print("",file=stderr)
return False


Expand All @@ -286,8 +283,9 @@ def execute_shell_script(db_path: Path, script_path: Path) -> bool:
)
return True
except subprocess.CalledProcessError as e:
console.print(f"[bold red]Error[/bold red] executing shell script {script_path}:")
console.print(e.stderr.decode(), style="red")
print(f"Error executing shell script {script_path}:", file=stderr)
sys.stderr.write(e.stderr.decode())
print("",file=stderr)
return False


Expand All @@ -306,7 +304,7 @@ def create_db_backup(db_path: Path) -> Path | None:
db_path = Path(db_path)
# Only proceed if the database exists
if not db_path.exists():
console.print(f"[yellow]Warning:[/yellow] Database file does not exist: {db_path}")
print(f"Warning: Database file does not exist: {db_path}")
return None

# Create a timestamped backup filename
Expand All @@ -315,7 +313,7 @@ def create_db_backup(db_path: Path) -> Path | None:

# Check if the backup file already exists
if backup_path.exists():
console.print(f"[red]Error:[/red] Backup file already exists: {backup_path}")
print(f"Error: Backup file already exists: {backup_path}", file=stderr)
return None

conn = None
Expand All @@ -332,17 +330,17 @@ def create_db_backup(db_path: Path) -> Path | None:
if not backup_path.exists():
raise Exception("Backup file was not created")

console.print(f"[green]Database backup created:[/green] {backup_path}")
print(f"Database backup created: {backup_path}")
return backup_path
except Exception as e:
console.print(f"[bold red]Error during backup:[/bold red] {e}")
print(f"Error during backup: {e}")
# Attempt to remove potentially incomplete backup file
if backup_path.exists():
try:
backup_path.unlink() # remove the file
console.print(f"[yellow]Removed incomplete backup file:[/yellow] {backup_path}")
print(f"Removed incomplete backup file: {backup_path}")
except OSError as remove_err:
console.print(f"[bold red]Error removing incomplete backup file:[/bold red] {remove_err}")
print(f"Error removing incomplete backup file: {remove_err}", file=stderr)
return None
finally:
# this runs before return `backup_path` or `return None` in the try block
Expand Down Expand Up @@ -373,7 +371,7 @@ def execute_migration_script(db_path: Path, script_path: Path) -> bool:
elif ext == ".sh":
return execute_shell_script(db_path, script_path)
else:
console.print(f"[bold red]Unsupported script type:[/bold red] {script_path}")
print(f"Unsupported script type: {script_path}", file=stderr)
return False


Expand All @@ -400,32 +398,29 @@ def run_migrations(
# Keep track of migration statistics
stats = {
"applied": 0,
"failed": 0,
"total_time": 0.0
"failed": 0
}

# Check if database file exists
if not db_path.exists():
console.print(f"[bold red]Error:[/bold red] Database file does not exist: {db_path}")
console.print("The database file must exist before running migrations.")
print(f"Error: Database file does not exist: {db_path}", file=stderr)
print("The database file must exist before running migrations.",file=stderr)
return False

try:
# Ensure this is a managed db
try:
create_db(db_path)
except sqlite3.Error as e:
console.print(f"""[bold red]Error:[/bold red] Cannot migrate the db at {db_path}.
print(f"""Error: Cannot migrate the db at {db_path}.

This is because it is not managed by fastmigrate. Please do one of the following:

1. Create a new, managed db using `fastmigrate.create_db()` or
`fastmigrate_create_db`

2. Enroll your existing database, by manually verifying your existing
db's data matches a version defined by your migration scripts, and
then setting your db's version explicitly with
`fastmigrate.core._set_db_version()`. See enrolling.md for guidance.""")
2. Enroll your existing database, as described in
https://answerdotai.github.io/fastmigrate/enrolling.html""",file=stderr)
return False

# Get current version
Expand All @@ -435,7 +430,7 @@ def run_migrations(
try:
migration_scripts = get_migration_scripts(migrations_dir)
except ValueError as e:
console.print(f"[bold red]Error:[/bold red] {e}")
print(f"Error: {e}", file=stderr)
return False

# Find pending migrations
Expand All @@ -447,63 +442,52 @@ def run_migrations(

if not pending_migrations:
if verbose:
console.print(f"[green]Database is up to date[/green] (version {current_version})")
print(f"Database is up to date (version {current_version})")
return True

# Sort migrations by version
sorted_versions = sorted(pending_migrations.keys())

# Execute migrations
start_time = time.time()
for version in sorted_versions:
script_path = pending_migrations[version]
script_name = script_path.name

# Start timing this migration
migration_start = time.time()
if verbose:
console.print(f"[blue]Applying[/blue] migration [bold]{version}[/bold]: [cyan]{script_name}[/cyan]")
print(f"Applying migration {version}: {script_name}")

# Each script will open its own connection

# Execute the migration script
success = execute_migration_script(db_path, script_path)

if not success:
console.print(f"[bold red]Migration failed:[/bold red] {script_path}")
stats["failed"] += 1

# Show summary of failure - always show errors regardless of verbose flag
console.print("\n[bold red]Migration Failed[/bold red]")
console.print(f" • [bold]{stats['applied']}[/bold] migrations applied")
console.print(f" • [bold]{stats['failed']}[/bold] migrations failed")
console.print(f" • Total time: [bold]{time.time() - start_time:.2f}[/bold] seconds")
stats["failed"] += 1
print(f"""Migration failed: {script_path}
• {stats['applied']} migrations applied
• {stats['failed']} migrations failed""", file=stderr)

return False

# Record migration duration
migration_duration = time.time() - migration_start
stats["total_time"] += migration_duration
stats["applied"] += 1

# Update version
_set_db_version(db_path, version)
if verbose:
console.print(f"[green]✓[/green] Database updated to version [bold]{version}[/bold] [dim]({migration_duration:.2f}s)[/dim]")
print(f"✓ Database updated to version {version}")

# Show summary of successful run
if stats["applied"] > 0 and verbose:
total_duration = time.time() - start_time
console.print("\n[bold green]Migration Complete[/bold green]")
console.print(f" • [bold]{stats['applied']}[/bold] migrations applied")
console.print(f" • Database now at version [bold]{sorted_versions[-1]}[/bold]")
console.print(f" • Total time: [bold]{total_duration:.2f}[/bold] seconds")
print("\nMigration Complete")
print(f" • {stats['applied']} migrations applied")
print(f" • Database now at version {sorted_versions[-1]}")

return True

except sqlite3.Error as e:
console.print(f"[bold red]Database error:[/bold red] {e}")
print(f"Database error: {e}", file=stderr)
return False
except Exception as e:
console.print(f"[bold red]Error:[/bold red] {e}")
print(f"Error: {e}", file=stderr)
return False
3 changes: 1 addition & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ readme = "README.md"
requires-python = ">=3.10"
license = {text = "MIT"}
dependencies = [
"apswutils>=0.0.2",
"rich>=14.0.0",
"apswutils>=0.0.2"
]

[dependency-groups]
Expand Down