Skip to content

Commit a43ab35

Browse files
refactor: remove autocommit checker, delegate to squawk config
The custom autocommit block checker duplicated what squawk handles natively via `assume_in_transaction` in `.squawk.toml`. Consumers configure this in their own repo, keeping the hook as a thin wrapper. Also consolidates version-check workflow into ci.yml and bumps to v0.3.0.
1 parent 3d188ad commit a43ab35

8 files changed

Lines changed: 56 additions & 339 deletions

File tree

.github/workflows/ci.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,15 @@ jobs:
3232
- name: Run pre-commit hooks
3333
run: poetry run pre-commit run --all-files
3434

35+
- name: Check version consistency
36+
run: |
37+
PYPROJECT_VERSION=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
38+
INIT_VERSION=$(grep '^__version__ = ' squawk_alembic/__init__.py | sed 's/__version__ = "\(.*\)"/\1/')
39+
if [ "$PYPROJECT_VERSION" != "$INIT_VERSION" ]; then
40+
echo "::error::Version mismatch: pyproject.toml ($PYPROJECT_VERSION) != __init__.py ($INIT_VERSION)"
41+
echo "Run 'make bump VERSION=x.y.z' to update both files."
42+
exit 1
43+
fi
44+
3545
- name: Run tests
3646
run: poetry run pytest tests/ -v

.github/workflows/version-check.yml

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

README.md

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@ A [pre-commit](https://pre-commit.com/) hook that lints SQL in [Alembic](https:/
44

55
Squawk operates on raw SQL files, but Alembic migrations are Python. This hook bridges the gap by generating DDL via `alembic upgrade --sql` (offline mode) and passing the complete SQL output to squawk for analysis. This captures all SQL statements a migration produces, including ORM operations like `op.create_index()`, `op.create_table()`, and `op.alter_column()`.
66

7-
The hook also checks that concurrent index operations (`CONCURRENTLY` in `op.execute()` or `postgresql_concurrently=True` in `op.create_index()` / `op.drop_index()`) are wrapped in `autocommit_block()`.
8-
97
## Usage
108

119
Add the following to your `.pre-commit-config.yaml`:
1210

1311
```yaml
1412
repos:
15-
- repo: https://github.com/kintsugi-tax/kintsugi-squawk
16-
rev: v0.2.0
13+
- repo: https://github.com/kintsugi-tax/squawk-pre-commit
14+
rev: v0.3.0
1715
hooks:
1816
- id: squawk-alembic
1917
```
@@ -26,9 +24,8 @@ When pre-commit runs, the hook:
2624

2725
1. Parses `alembic.ini` to find the migrations `versions/` directory
2826
2. Filters staged files to only those under that directory
29-
3. Checks for concurrent operations outside `autocommit_block()`
30-
4. Runs `alembic upgrade --sql` to generate the complete DDL for each migration
31-
5. Pipes the generated SQL to squawk for linting
27+
3. Runs `alembic upgrade --sql` to generate the complete DDL for each migration
28+
4. Pipes the generated SQL to squawk for linting
3229

3330
Merge migrations (where `down_revision` is a tuple) are skipped since they produce no DDL.
3431

@@ -55,5 +52,5 @@ To test the hook against a consumer repo locally:
5552

5653
```bash
5754
cd /path/to/consumer-repo
58-
pre-commit try-repo /path/to/kintsugi-squawk squawk-alembic --all-files
55+
pre-commit try-repo /path/to/squawk-pre-commit squawk-alembic --all-files
5956
```

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"
44

55
[tool.poetry]
66
name = "squawk-alembic"
7-
version = "0.2.0"
7+
version = "0.3.0"
88
description = "Pre-commit hook to lint Alembic migration SQL with squawk"
99
packages = [{include = "squawk_alembic"}]
1010

squawk_alembic/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.2.0"
1+
__version__ = "0.3.0"

squawk_alembic/hook.py

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -126,86 +126,6 @@ def generate_sql(filepath):
126126
return result.stdout
127127

128128

129-
def check_autocommit_blocks(filepath):
130-
"""Check that CONCURRENTLY operations are inside autocommit blocks."""
131-
with open(filepath) as f:
132-
try:
133-
tree = ast.parse(f.read())
134-
except SyntaxError:
135-
return []
136-
137-
checker = _AutocommitChecker()
138-
checker.visit(tree)
139-
return checker.warnings
140-
141-
142-
def _has_concurrent_kwarg(node):
143-
"""Check if an AST Call node has postgresql_concurrently=True."""
144-
for kw in node.keywords:
145-
if (
146-
kw.arg == "postgresql_concurrently"
147-
and isinstance(kw.value, ast.Constant)
148-
and kw.value.value is True
149-
):
150-
return True
151-
return False
152-
153-
154-
class _AutocommitChecker(ast.NodeVisitor):
155-
def __init__(self):
156-
self.warnings = []
157-
self._in_autocommit = False
158-
159-
def visit_With(self, node):
160-
is_autocommit = any(
161-
isinstance(item.context_expr, ast.Call)
162-
and isinstance(item.context_expr.func, ast.Attribute)
163-
and item.context_expr.func.attr == "autocommit_block"
164-
for item in node.items
165-
)
166-
if is_autocommit:
167-
old = self._in_autocommit
168-
self._in_autocommit = True
169-
self.generic_visit(node)
170-
self._in_autocommit = old
171-
else:
172-
self.generic_visit(node)
173-
174-
def visit_Call(self, node):
175-
if (
176-
isinstance(node.func, ast.Attribute)
177-
and isinstance(node.func.value, ast.Name)
178-
and node.func.value.id == "op"
179-
):
180-
# op.execute("...CONCURRENTLY...")
181-
if node.func.attr == "execute" and node.args:
182-
sql = _extract_string(node.args[0])
183-
if sql and "concurrently" in sql.lower() and not self._in_autocommit:
184-
self.warnings.append(node.lineno)
185-
186-
# op.create_index(..., postgresql_concurrently=True)
187-
# op.drop_index(..., postgresql_concurrently=True)
188-
if node.func.attr in ("create_index", "drop_index"):
189-
if _has_concurrent_kwarg(node) and not self._in_autocommit:
190-
self.warnings.append(node.lineno)
191-
192-
self.generic_visit(node)
193-
194-
195-
def _extract_string(node):
196-
"""Extract a string value from an AST node, handling common wrappers."""
197-
if isinstance(node, ast.Constant) and isinstance(node.value, str):
198-
return node.value
199-
200-
if isinstance(node, ast.Call) and node.args:
201-
if isinstance(node.func, ast.Attribute) and node.func.attr == "text":
202-
return _extract_string(node.args[0])
203-
if isinstance(node.func, ast.Name) and node.func.id == "text":
204-
return _extract_string(node.args[0])
205-
206-
return None
207-
208-
209129
def main():
210130
files = sys.argv[1:]
211131
if not files:
@@ -227,13 +147,6 @@ def main():
227147
except ValueError:
228148
continue
229149

230-
autocommit_warnings = check_autocommit_blocks(filepath)
231-
for lineno in autocommit_warnings:
232-
print(
233-
f"{filepath}:{lineno}: CONCURRENTLY operation outside autocommit_block()"
234-
)
235-
exit_code = 1
236-
237150
sql = generate_sql(filepath)
238151
if not sql:
239152
continue

0 commit comments

Comments
 (0)