-
Notifications
You must be signed in to change notification settings - Fork 523
Feat/add optional prek prepush #3997
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.
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
Open
tetelio
wants to merge
15
commits into
devel
Choose a base branch
from
feat/add-optional-prek-prepush
base: devel
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,152
−4
Open
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
284058c
Add prek checks
tetelio 038bd47
Add better structure of commands
tetelio 63e8e02
Better code orga
tetelio 0c68673
Fix typing
tetelio 7a71894
FL
tetelio 7a04500
Fix makefile and contributing
tetelio 3c295c0
Update toml
tetelio aa5b630
Use tomli to support 3.10
tetelio 15ba2a0
ADdd tomli to uv lock from docs too
tetelio 0f1999f
Parallel linting with make fl
tetelio 1d624e1
Move config to pyproject.toml. Add multiline cache
tetelio 2e0f2ae
Fix docs
tetelio 96708d4
FL
tetelio 9363cf9
Rename scopes
tetelio a2ea0ed
FL
tetelio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| # Prek automatic checks | ||
| .prek/local.toml | ||
| .prek/.state.toml | ||
| .prek/.enabled | ||
|
|
||
| # Claude Code | ||
| .worktrees | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # Pre-push local verification | ||
|
|
||
| Optional pre-push checks via [prek](https://github.com/pre-commit/prek): run `make fl` and/or | ||
| `make test-common-p` before you push, but only when files in each check’s scope have changed since | ||
| the last successful run. | ||
|
|
||
| The hook is **opt-in**. Without `.prek/local.toml`, the pre-push hook does nothing. | ||
|
|
||
| ## Quick start | ||
|
|
||
| 1. Copy `local.example.toml` to `local.toml` and set each check `mode` (`off`, `auto`, or `confirm`). | ||
| 2. From the repo root: `make install-prepush-hooks` | ||
| 3. Push as usual. Bypass once: `git push --no-verify` | ||
| 4. Preview without pushing: `make prek-dry`. Run the gate manually: `make prek` | ||
| 5. Remove the hook: `make uninstall-prepush-hooks` | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| - Root dev env: `make dev` (for `make fl` and `make test-common-p`) | ||
| - Docs Python env: `cd docs && make dev` (once). `make fl` also runs `npm install` in `docs/website` for Biome. | ||
| - Optional `[gate] only_when_pr_open = true`: requires [GitHub CLI](https://cli.github.com/) (`gh auth login`) | ||
|
|
||
| ## Configuration (`local.toml`) | ||
|
|
||
| Copy from `local.example.toml`. Gitignored — per-developer only. | ||
|
|
||
| | Section | Keys | Meaning | | ||
| |---------|------|---------| | ||
| | `[gate]` | `only_when_pr_open` | If `true`, skip all checks unless the current branch has an open PR (`gh pr view`) | | ||
| | `[lint]` | `mode` | How to handle stale lint scope | | ||
| | `[test_common_p]` | `mode` | How to handle stale common-test scope | | ||
|
|
||
| ### Modes | ||
|
|
||
| | Mode | When scope is stale | | ||
| |------|---------------------| | ||
| | `off` | Never run this check | | ||
| | `auto` | Run the make target | | ||
| | `confirm` | Ask on the terminal; declining aborts the push | | ||
|
|
||
| **Confirm prompts** look like: `Run make fl before push? [Y/n] ` | ||
|
|
||
| - Enter or `y` / `yes` → run the check | ||
| - `n` / `no` → abort push | ||
| - Non-interactive stdin (no TTY) → treated as declined (push blocked) | ||
|
|
||
| ## What runs | ||
|
|
||
| Checks run in order; a failed lint blocks tests. | ||
|
|
||
| | Check | Make target | Command recorded in state | | ||
| |-------|-------------|---------------------------| | ||
| | `lint` | `fl` | `make fl` | | ||
| | `test_common_p` | `test-common-p` | `make test-common-p` | | ||
|
|
||
| `make fl` runs format (root, docs, website deps) in parallel, then root and docs lint in parallel. | ||
|
|
||
| A check runs only when its **fingerprint** (hash of tracked files in scope) differs from the last | ||
| successful entry in `.prek/.state.toml` (also gitignored). Passing updates state for that check only. | ||
|
|
||
| After `make install-prepush-hooks`, successful `make fl` and `make test-common-p` also update | ||
| state (no extra commands). Plain `make lint` does not update prek state. | ||
|
|
||
| ## Unstaged changes on push | ||
|
|
||
| prek may stash unstaged edits to `~/.cache/prek/patches/` while the hook runs, then restore them. | ||
| Built-in prek behavior (from pre-commit), not configurable. Keeps lint/tests from failing on WIP you | ||
| are not pushing. | ||
|
|
||
| ## Scopes (`scopes.toml`) | ||
|
|
||
| Defines which tracked files invalidate each check. Edit when adding new trees that should trigger | ||
| re-lint or re-test. | ||
|
|
||
| **Lint** — `dlt`, `tests`, `tools`, `docs` (`.py`, `.md`, `.ipynb`), plus root/docs config and | ||
| embedded-snippet lint setup files. | ||
|
|
||
| **Common tests** — `dlt` and selected `tests/*` suites (see `scopes.toml`), plus `pyproject.toml`, | ||
| `uv.lock`, `tests/conftest.py`, `tests/load/test_dummy_client.py`. | ||
|
|
||
| Inspect a fingerprint: | ||
|
|
||
| ```bash | ||
| uv run python -m tools.prek fingerprint lint | ||
| uv run python -m tools.prek fingerprint test_common_p | ||
| ``` | ||
|
|
||
| ## Makefile targets | ||
|
|
||
| | Target | Purpose | | ||
| |--------|---------| | ||
| | `make install-prepush-hooks` | Install prek pre-push hook and enable state recording (fails if another pre-push hook exists) | | ||
| | `make uninstall-prepush-hooks` | Remove the prek pre-push hook (no-op if none; fails if hook is not from prek) | | ||
| | `make prek` | Run the gate now (same logic as on push) | | ||
| | `make prek-dry` | Show what would run; no make, no state update | | ||
| | `make fl` | Format root + docs (parallel), then lint root + docs (parallel) | | ||
|
|
||
| prek is installed with `uv tool install`, not as a repo dependency. | ||
|
|
||
| ## Troubleshooting | ||
|
|
||
| **Existing pre-push hook** — `make install-prepush-hooks` refuses to install if `.git/hooks/pre-push` | ||
| already exists and is not from prek. prek cannot share the hook file with another tool. Remove or | ||
| relocate your hook first, or skip prek setup for now. | ||
|
|
||
| **Uninstall with a foreign hook** — `make uninstall-prepush-hooks` only removes a prek-managed hook. | ||
| If `.git/hooks/pre-push` exists but was not installed via `make install-prepush-hooks`, uninstall | ||
| refuses to run so your hook is not deleted. | ||
|
|
||
| **Hook never runs checks** — Ensure `.prek/local.toml` exists and at least one check has `mode` not | ||
| `off`. Run `make prek-dry` to see whether the gate is active and which checks are stale. | ||
|
|
||
| **Gate skipped** — With `only_when_pr_open = true`, there must be an open PR on the current branch. | ||
|
|
||
| **Docs lint fails** — Run `cd docs && make dev`, then `make fl` (or `cd docs && make format && make lint`). | ||
|
|
||
| **Want to re-run after a pass** — Delete the check’s section from `.prek/.state.toml`, or change a | ||
| file in that check’s scope. | ||
|
|
||
| **Stale fingerprint / wrong cache** — Same as above; state stores the last successful fingerprint | ||
| per check. | ||
|
|
||
| ## Files in this directory | ||
|
|
||
| | File | Role | | ||
| |------|------| | ||
| | `README.md` | This guide | | ||
| | `local.example.toml` | Config template | | ||
| | `scopes.toml` | Fingerprint inputs per check | | ||
| | `prek.toml` | prek hook definition (`uv run python -m tools.prek`) | | ||
| | `plan.md` | Maintainer notes / design sketch | | ||
|
|
||
| Implementation and tests: `tools/prek.py` (run via `python -m tools.prek`). | ||
|
|
||
| Gitignored: `local.toml`, `.state.toml`, `.enabled` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| [gate] | ||
| only_when_pr_open = false # if true, run checks only when the current branch has an open PR | ||
|
|
||
| [lint] | ||
| mode = "auto" # off | auto | confirm | ||
|
|
||
| [test_common_p] | ||
| mode = "off" # off | auto | confirm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| [[repos]] | ||
| repo = "local" | ||
|
|
||
| [[repos.hooks]] | ||
| id = "pre-push-gate" | ||
| name = "dlt pre-push gate" | ||
| language = "system" | ||
| entry = "uv run python -m tools.prek" | ||
| pass_filenames = false | ||
| always_run = true | ||
| stages = ["pre-push"] |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| [scopes.lint] | ||
| paths = ["dlt", "tests", "tools", "docs"] | ||
| globs = ["*.py", "*.md", "*.ipynb"] | ||
| files = [ | ||
| "pyproject.toml", | ||
| "uv.lock", | ||
| "docs/pyproject.toml", | ||
| "docs/uv.lock", | ||
| "docs/docs_tools/snippets/lint_setup/template.py", | ||
| "docs/docs_tools/snippets/lint_setup/mypy.ini", | ||
| ] | ||
|
|
||
| [scopes.test_common_p] | ||
| paths = [ | ||
| "dlt", | ||
| "tests/common", | ||
| "tests/normalize", | ||
| "tests/extract", | ||
| "tests/pipeline", | ||
| "tests/reflection", | ||
| "tests/sources", | ||
| "tests/workspace", | ||
| "tests/libs", | ||
| "tests/destinations", | ||
| ] | ||
| globs = ["*.py"] | ||
| files = [ | ||
| "pyproject.toml", | ||
| "uv.lock", | ||
| "tests/conftest.py", | ||
| "tests/load/test_dummy_client.py", | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| .DEFAULT_GOAL := help | ||
| .PHONY: install-uv has-uv dev lint test test-common test-common-p reset-test-storage recreate-compiled-deps build-library-prerelease build-library publish-library test-load-local test-load-local-p test-load-local-postgres test-load-local-postgres-p install-snowflake-extras test-remote-snowflake test-remote-snowflake-p install-common-core test-common-core install-common-core-source test-common-core-source install-common-source install-pipeline-min test-pipeline-min install-pipeline-arrow test-pipeline-arrow install-pipeline-min-arrow test-pipeline-min-arrow install-workspace test-workspace test-workspace-dashboard install-hub-minimal test-hub-minimal test-hub install-pipeline-full test-pipeline-full install-pipeline-full-sql test-pipeline-full-sql install-sqlalchemy2 test-with-sqlalchemy-2 test-dest-load test-dest-remote-essential test-dest-remote-nonessential test-dbt-no-venv test-dbt-runner-venv test-sources-load test-sources-sql-database | ||
| .PHONY: install-uv has-uv dev lint lint-parallel lint-full lint-docs format format-docs docs-website-deps fl test test-common test-common-p reset-test-storage recreate-compiled-deps build-library-prerelease build-library publish-library test-load-local test-load-local-p test-load-local-postgres test-load-local-postgres-p install-snowflake-extras test-remote-snowflake test-remote-snowflake-p install-common-core test-common-core install-common-core-source test-common-core-source install-common-source install-pipeline-min test-pipeline-min install-pipeline-arrow test-pipeline-arrow install-pipeline-min-arrow test-pipeline-min-arrow install-workspace test-workspace test-workspace-dashboard install-hub-minimal test-hub-minimal test-hub install-pipeline-full test-pipeline-full install-pipeline-full-sql test-pipeline-full-sql install-sqlalchemy2 test-with-sqlalchemy-2 test-dest-load test-dest-remote-essential test-dest-remote-nonessential test-dbt-no-venv test-dbt-runner-venv test-sources-load test-sources-sql-database install-prepush-hooks uninstall-prepush-hooks prek prek-dry | ||
|
|
||
| PYV=$(shell python3 -c "import sys;t='{v[0]}.{v[1]}'.format(v=list(sys.version_info[:2]));sys.stdout.write(t)") | ||
| .SILENT:has-uv | ||
|
|
@@ -37,7 +37,30 @@ dev-airflow: has-uv ## Prepares development environment with airflow support | |
| dev-hub: has-uv ## Prepares development environment with hub support | ||
| uv sync --all-extras --group workspace-deps --group dev --group providers --group pipeline --group sources --group sentry-sdk --group ibis --group adbc --group dashboard-tests | ||
|
|
||
| lint: lint-core lint-security lint-docstrings lint-lock lint-deps ## Runs all linters (mypy, ruff, flake8, bandit, docstrings, lockfile, deps) | ||
| LINT_TARGETS := lint-core lint-security lint-docstrings lint-lock lint-deps | ||
|
|
||
| lint: $(LINT_TARGETS) ## Runs all linters (mypy, ruff, flake8, bandit, docstrings, lockfile, deps) | ||
| @: | ||
|
|
||
| lint-parallel: ## Runs all linters in parallel (used by make fl) | ||
| $(MAKE) -j $(words $(LINT_TARGETS)) lint | ||
|
|
||
| lint-full: lint lint-docs ## Root + docs lint (sequential) | ||
|
|
||
| lint-docs: ## Runs docs linting (embedded snippets, notebooks, docs tooling) | ||
| cd docs && $(MAKE) lint | ||
|
|
||
| format-docs: ## Formats docs tooling, website, examples, and notebooks | ||
| cd docs && $(MAKE) format | ||
|
|
||
| docs-website-deps: ## Install docs website node deps (biome; used by make fl) | ||
| cd docs/website && npm install | ||
|
|
||
| fl: ## Format then lint root and docs in parallel (prek pre-push gate) | ||
| set -e; \ | ||
| $(MAKE) format & $(MAKE) format-docs & $(MAKE) docs-website-deps & wait; \ | ||
| $(MAKE) lint-parallel & $(MAKE) -C docs lint-parallel & wait | ||
| @if [ -f .prek/.enabled ]; then uv run python -m tools.prek --record lint; fi | ||
|
|
||
| lint-lock: ## Checks uv lockfile is in sync | ||
| uv lock --check | ||
|
|
@@ -151,11 +174,13 @@ TEST_COMMON_PATHS = \ | |
| tests/libs \ | ||
| tests/destinations | ||
|
|
||
| test-common: PYTEST_MARKERS = not rfam | ||
| test-common: ## Tests common components without external resources | ||
| $(call RUN_XDIST_SAFE_SPLIT,$(TEST_COMMON_PATHS)) | ||
|
|
||
| test-common-p: ## Tests common components in parallel | ||
| $(MAKE) test-common PYTEST_XDIST_N=auto | ||
| @if [ -f .prek/.enabled ]; then uv run python -m tools.prek --record test_common_p; fi | ||
|
|
||
| # ---------------------------------------------------------------------- | ||
| # Local load tests | ||
|
|
@@ -432,3 +457,34 @@ test-e2e-dashboard-headed: ## Runs dashboard e2e tests with visible browser | |
|
|
||
| create-test-pipelines: ## Creates test pipelines for manual dashboard testing | ||
| uv run python tests/workspace/helpers/dashboard/example_pipelines.py | ||
|
|
||
| PREK_VERSION ?= 0.4.2 | ||
|
|
||
| prek: ## Run pre-push gate now (same as the git hook) | ||
| uv run python -m tools.prek | ||
|
|
||
| prek-dry: ## Show what the pre-push gate would run (no make, no state update) | ||
| uv run python -m tools.prek --dry-run | ||
|
|
||
| install-prepush-hooks: ## Install prek pre-push hook (fails if another pre-push hook exists) | ||
| @if [ -f .git/hooks/pre-push ] && ! grep -Fq 'File generated by prek' .git/hooks/pre-push 2>/dev/null; then \ | ||
| echo "Error: .git/hooks/pre-push already exists."; \ | ||
| echo "prek is not compatible with an existing pre-push hook."; \ | ||
| echo "Remove or relocate your hook, then run make install-prepush-hooks again."; \ | ||
| exit 1; \ | ||
| fi | ||
| uv tool install prek@$(PREK_VERSION) | ||
| prek install --hook-type pre-push --config .prek/prek.toml -f | ||
| @touch .prek/.enabled | ||
|
|
||
| uninstall-prepush-hooks: ## Remove prek pre-push hook (no-op if none; fails if hook is not from prek) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| @if [ ! -f .git/hooks/pre-push ]; then \ | ||
| echo "No pre-push hook to remove."; \ | ||
| elif ! grep -Fq 'File generated by prek' .git/hooks/pre-push 2>/dev/null; then \ | ||
| echo "Error: .git/hooks/pre-push exists but was not installed by make install-prepush-hooks."; \ | ||
| echo "make uninstall-prepush-hooks will not remove it."; \ | ||
| exit 1; \ | ||
| else \ | ||
| prek uninstall --hook-type pre-push --config .prek/prek.toml; \ | ||
| fi | ||
| @rm -f .prek/.enabled | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration can be defined in
pyproject.toml. This would reduce file sprawl.If devs want to override stuff locally, then can:
.git/); this PR will have no effect on them