Skip to content

Commit

Permalink
Merge pull request #32 from d-ganchar/31-query_settings
Browse files Browse the repository at this point in the history
#31 query_settings, dependency versions, test.yml
  • Loading branch information
vinayak-mehta authored Jan 12, 2025
2 parents d752594 + 780df34 commit 9405895
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 68 deletions.
42 changes: 22 additions & 20 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
jobs:
test:
runs-on: ubuntu-latest

services:
clickhouse:
image: clickhouse/clickhouse-server:latest
Expand All @@ -20,23 +20,25 @@ jobs:
--health-timeout 5s
--health-retries 5
strategy:
fail-fast: false
matrix:
python-version: [ "3.10", "3.11", "3.12", "3.13" ]

env:
UV_PYTHON: ${{ matrix.python-version }}

steps:
- uses: actions/checkout@v3

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -e '.[dev]'
- name: Check formatting with Ruff
run: |
ruff format --check .
- name: Run tests
run: |
pytest tests/ -v --cov=houseplant
- uses: actions/checkout@v3
- uses: astral-sh/setup-uv@v5
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: uv sync --all-extras

- name: Check formatting with Ruff
run: ruff check

- name: Run tests
run: pytest tests/ -v --cov=houseplant
26 changes: 12 additions & 14 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ maintainers = [{name = "June", email = "[email protected]"}]
license = {text = "MIT license"}
classifiers = [
"License :: OSI Approved :: MIT License",
"Topic :: Database",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
]
dependencies = [
"clickhouse-driver>=0.2.9",
"pyyaml>=6.0.2",
"ruff>=0.7.4",
"typer>=0.14.0",
"python-dotenv>=1.0.1",
"clickhouse-driver >= 0.2.9, < 0.3",
"pyyaml >= 6.0.2, < 6.1",
"typer >= 0.14.0, < 0.15",
"python-dotenv >= 1.0.1, < 1.1",
]

[build-system]
Expand All @@ -35,13 +36,10 @@ houseplant = "houseplant.cli:app"

[project.optional-dependencies]
dev = [
"coverage", # testing
"mypy", # linting
"pytest", # testing
"pytest-cov", # testing
"pytest-mock", # mocking
"ruff", # linting
"isort", # linting
"sphinx", # documentation
"tomli", # documentation
"ruff==0.8.6", # linting
"pytest==8.3.4", # testing
"pytest-cov==6.0.0", # testing
"pytest-mock==3.14.0", # testing
"sphinx", # documentation
"tomli", # documentation
]
5 changes: 2 additions & 3 deletions src/houseplant/clickhouse_client.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""ClickHouse database operations module."""

import os
import sys

from clickhouse_driver import Client
from clickhouse_driver.errors import NetworkError, ServerException
Expand Down Expand Up @@ -235,12 +234,12 @@ def get_applied_migrations(self):
ORDER BY version
""")

def execute_migration(self, sql: str):
def execute_migration(self, sql: str, query_settings: dict = None):
"""Execute a migration SQL statement."""
# Split multiple statements and execute them separately
statements = [stmt.strip() for stmt in sql.split(";") if stmt.strip()]
for statement in statements:
self.client.execute(statement)
self.client.execute(statement, settings=query_settings)

def mark_migration_applied(self, version: str):
"""Mark a migration as applied."""
Expand Down
38 changes: 18 additions & 20 deletions src/houseplant/houseplant.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,7 @@ def migrate_up(self, version: str | None = None):
with open(os.path.join(MIGRATIONS_DIR, migration_file), "r") as f:
migration = yaml.safe_load(f)

# Get migration SQL based on environment
migration_sql = migration.get(self.env, {}).get("up", {}).strip()

table = migration.get("table", "").strip()

if not table:
self.console.print(
"[red]✗[/red] Migration [bold red]failed[/bold red]: "
Expand Down Expand Up @@ -144,9 +140,12 @@ def migrate_up(self, version: str | None = None):
}
)

migration_sql = migration_sql.format(**format_args).strip()
# Get migration SQL based on environment
migration_env: dict = migration.get(self.env, {})
migration_sql = migration_env.get("up", "").format(**format_args).strip()

if migration_sql:
self.db.execute_migration(migration_sql)
self.db.execute_migration(migration_sql, migration_env.get("query_settings"))
self.db.mark_migration_applied(migration_version)
self.console.print(
f"[green]✓[/green] Applied migration {migration_file}"
Expand Down Expand Up @@ -200,28 +199,27 @@ def migrate_down(self, version: str | None = None):
with open(os.path.join(MIGRATIONS_DIR, migration_file), "r") as f:
migration = yaml.safe_load(f)

# Get migration SQL based on environment
migration_sql = migration.get(self.env, {}).get("down", {}).strip()
table = migration.get("table", "").strip()
if not table:
self.console.print(
"[red]✗[/red] [bold red] Migration failed[/bold red]: "
"'table' field is required in migration file"
)
return
migration_sql = migration_sql.format(table=table).strip()

# Get migration SQL based on environment
migration_env = migration.get(self.env, {})
migration_sql = migration_env.get("down", {}).format(table=table).strip()

if migration_sql:
self.db.execute_migration(migration_sql)
self.db.execute_migration(migration_sql, migration_env.get('query_settings'))
self.db.mark_migration_rolled_back(migration_version)
self.update_schema()
self.console.print(
f"[green]✓[/green] Rolled back migration {migration_file}"
)
else:
self.console.print(
f"[yellow]⚠[/yellow] Empty down migration {migration_file}"
)
self.console.print(f"[green]✓[/green] Rolled back migration {migration_file}")

return

self.console.print(f"[yellow]⚠[/yellow] Empty down migration {migration_file}")

def migrate(self, version: str | None = None):
"""Run migrations up to specified version."""
Expand Down Expand Up @@ -335,9 +333,9 @@ def update_schema(self):
processed_tables.add(table_name)

# Finally dictionaries
for dict in dictionaries:
if dict[0] == table_name:
dict_name = dict[0]
for ch_dict in dictionaries:
if ch_dict[0] == table_name:
dict_name = ch_dict[0]
create_stmt = self.db.client.execute(
f"SHOW CREATE DICTIONARY {dict_name}"
)[0][0]
Expand Down
4 changes: 1 addition & 3 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import os
from pathlib import Path
from typing import Generator

import pytest
from typer.testing import CliRunner

from houseplant import Houseplant, __version__
from houseplant.cli import app
from typer.testing import CliRunner

runner = CliRunner()

Expand Down
67 changes: 59 additions & 8 deletions tests/test_houseplant.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,51 @@ def duplicate_migrations(tmp_path):
return ("20240101000000", "20240102000000")


@pytest.fixture
def migration_with_settings(tmp_path):
migrations_dir = tmp_path / "ch/migrations"
migrations_dir.mkdir(parents=True)
migration_file = migrations_dir / "20240101000000_test_migration_with_settings.yml"

migration_content = """version: "20240101000000"
name: test_migration_with_settings
table: dynamic_type_table
development:
query_settings:
enable_dynamic_type: 1
max_table_size_to_drop: 0
up: CREATE TABLE {table} (d Dynamic) ENGINE = MergeTree() ORDER BY d
down: DROP TABLE {table}
"""

migration_file.write_text(migration_content)
os.chdir(tmp_path)
return migration_content


def test_migration_with_settings(houseplant, migration_with_settings, mocker):
mock_execute = mocker.patch.object(houseplant.db.client, 'execute')
settings = {'settings': {'enable_dynamic_type': 1, 'max_table_size_to_drop': 0}}

houseplant.migrate_up()
assert list(mock_execute.call_args_list[1]) == [
('CREATE TABLE dynamic_type_table (d Dynamic) ENGINE = MergeTree() ORDER BY d', ),
settings,
]

mocker.patch.object(houseplant.db, "mark_migration_rolled_back")
mocker.patch.object(
houseplant.db, "get_applied_migrations", return_value=[("20240101000000",)]
)

houseplant.migrate_down()
assert list(mock_execute.call_args_list[4]) == [
('DROP TABLE dynamic_type_table',),
settings,
]


def test_migrate_up_development(houseplant, test_migration, mocker):
# Mock environment and database calls
houseplant.env = "development"
Expand All @@ -222,7 +267,8 @@ def test_migrate_up_development(houseplant, test_migration, mocker):
name String
) ENGINE = MergeTree()
ORDER BY id"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand All @@ -245,7 +291,8 @@ def test_migrate_up_production(houseplant, test_migration, mocker):
name String
) ENGINE = ReplicatedMergeTree()
ORDER BY id"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand All @@ -272,7 +319,8 @@ def test_migrate_up_with_development_table(
) ENGINE = MergeTree()
ORDER BY (id, created_at)
PARTITION BY toYYYYMM(created_at)"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand All @@ -299,7 +347,8 @@ def test_migrate_up_with_production_table(
) ENGINE = MergeTree()
ORDER BY (id, created_at)
PARTITION BY toYYYYMM(created_at)"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand All @@ -324,7 +373,8 @@ def test_migrate_up_with_development_view(houseplant, test_migration_with_view,
created_at DateTime
)
AS SELECT * FROM events"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand All @@ -350,7 +400,8 @@ def test_migrate_up_with_production_view(houseplant, test_migration_with_view, m
created_at DateTime
)
AS SELECT * FROM events"""
mock_execute.assert_called_once_with(expected_sql)

mock_execute.assert_called_once_with(expected_sql, None)
mock_mark_applied.assert_called_once_with("20240101000000")
mock_get_applied.assert_called_once()

Expand Down Expand Up @@ -443,8 +494,8 @@ def test_migrate_up_missing_version(houseplant, test_migration, mocker):

def test_migrate_up_missing_table_field(houseplant, tmp_path, mocker):
# Mock database calls
mock_execute = mocker.patch.object(houseplant.db, "execute_migration")
mock_mark_applied = mocker.patch.object(houseplant.db, "mark_migration_applied")
mocker.patch.object(houseplant.db, "execute_migration")
mocker.patch.object(houseplant.db, "mark_migration_applied")
mock_get_applied = mocker.patch.object(
houseplant.db, "get_applied_migrations", return_value=[]
)
Expand Down

0 comments on commit 9405895

Please sign in to comment.