Skip to content

Commit c112050

Browse files
authored
Pre commit Hooks (black, flake8, mypy, etc) [CT-105] (dbt-labs#4639)
Added pre-commit and configured hooks (black, flake8, mypy, white space formatters) Removed code checks from tox updated CI
1 parent 43e3fc2 commit c112050

File tree

9 files changed

+148
-69
lines changed

9 files changed

+148
-69
lines changed

.flake8

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[flake8]
2+
select =
3+
E
4+
W
5+
F
6+
ignore =
7+
W503 # makes Flake8 work like black
8+
W504
9+
E203 # makes Flake8 work like black
10+
E741
11+
max-line-length = 99
12+
exclude = test

.github/workflows/main.yml

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,10 @@ defaults:
3737

3838
jobs:
3939
code-quality:
40-
name: ${{ matrix.toxenv }}
40+
name: code-quality
4141

4242
runs-on: ubuntu-latest
4343

44-
strategy:
45-
fail-fast: false
46-
matrix:
47-
toxenv: [flake8, mypy]
48-
49-
env:
50-
TOXENV: ${{ matrix.toxenv }}
51-
PYTEST_ADDOPTS: "-v --color=yes"
52-
5344
steps:
5445
- name: Check out the repository
5546
uses: actions/checkout@v2
@@ -62,12 +53,16 @@ jobs:
6253
- name: Install python dependencies
6354
run: |
6455
pip install --user --upgrade pip
65-
pip install tox
56+
pip install pre-commit
57+
pip install mypy==0.782
58+
pip install -r editable-requirements.txt
6659
pip --version
67-
tox --version
60+
pre-commit --version
61+
mypy --version
62+
dbt --version
6863
69-
- name: Run tox
70-
run: tox
64+
- name: Run pre-commit hooks
65+
run: pre-commit run --all-files --show-diff-on-failure
7166

7267
unit:
7368
name: unit test / python ${{ matrix.python-version }}

.github/workflows/release.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ jobs:
142142
run: |
143143
dbt --version
144144
145-
146145
github-release:
147146
name: GitHub Release
148147

.gitignore

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,8 @@ coverage.xml
4949
*,cover
5050
.hypothesis/
5151
test.env
52+
*.pytest_cache/
5253

53-
# Mypy
54-
.mypy_cache/
5554

5655
# Translations
5756
*.mo
@@ -66,10 +65,10 @@ docs/_build/
6665
# PyBuilder
6766
target/
6867

69-
#Ipython Notebook
68+
# Ipython Notebook
7069
.ipynb_checkpoints
7170

72-
#Emacs
71+
# Emacs
7372
*~
7473

7574
# Sublime Text
@@ -78,6 +77,7 @@ target/
7877
# Vim
7978
*.sw*
8079

80+
# Pyenv
8181
.python-version
8282

8383
# Vim
@@ -90,6 +90,7 @@ venv/
9090
# AWS credentials
9191
.aws/
9292

93+
# MacOS
9394
.DS_Store
9495

9596
# vscode

.pre-commit-config.yaml

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Configuration for pre-commit hooks (see https://pre-commit.com/).
2+
# Eventually the hooks described here will be run as tests before merging each PR.
3+
4+
# TODO: remove global exclusion of tests when testing overhaul is complete
5+
exclude: ^test/
6+
7+
# Force all unspecified python hooks to run python 3.8
8+
default_language_version:
9+
python: python3.8
10+
11+
repos:
12+
- repo: https://github.com/pre-commit/pre-commit-hooks
13+
rev: v3.2.0
14+
hooks:
15+
- id: check-yaml
16+
args: [--unsafe]
17+
- id: check-json
18+
- id: end-of-file-fixer
19+
- id: trailing-whitespace
20+
- id: check-case-conflict
21+
- repo: https://github.com/psf/black
22+
rev: 21.12b0
23+
hooks:
24+
- id: black
25+
args:
26+
- "--line-length=99"
27+
- "--target-version=py38"
28+
- id: black
29+
alias: black-check
30+
stages: [manual]
31+
args:
32+
- "--line-length=99"
33+
- "--target-version=py38"
34+
- "--check"
35+
- "--diff"
36+
- repo: https://gitlab.com/pycqa/flake8
37+
rev: 4.0.1
38+
hooks:
39+
- id: flake8
40+
- id: flake8
41+
alias: flake8-check
42+
stages: [manual]
43+
- repo: https://github.com/pre-commit/mirrors-mypy
44+
rev: v0.782
45+
hooks:
46+
- id: mypy
47+
# N.B.: Mypy is... a bit fragile.
48+
#
49+
# By using `language: system` we run this hook in the local
50+
# environment instead of a pre-commit isolated one. This is needed
51+
# to ensure mypy correctly parses the project.
52+
53+
# It may cause trouble
54+
# in that it adds environmental variables out of our control to the
55+
# mix. Unfortunately, there's nothing we can do about per pre-commit's
56+
# author.
57+
# See https://github.com/pre-commit/pre-commit/issues/730 for details.
58+
args: [--show-error-codes]
59+
files: ^core/dbt/
60+
language: system
61+
- id: mypy
62+
alias: mypy-check
63+
stages: [manual]
64+
args: [--show-error-codes, --pretty]
65+
files: ^core/dbt/
66+
language: system

CONTRIBUTING.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Here's a good workflow:
4747
- Outline your planned implementation. If you want help getting started, ask!
4848
- Follow the steps outlined below to develop locally. Once you have opened a PR, one of the `dbt-core` maintainers will work with you to review your code.
4949
- Add a test! Tests are crucial for both fixes and new features alike. We want to make sure that code works as intended, and that it avoids any bugs previously encountered. Currently, the best resource for understanding `dbt-core`'s [unit](test/unit) and [integration](test/integration) tests is the tests themselves. One of the maintainers can help by pointing out relevant examples.
50+
- Check your formatting and linting with [Flake8](https://flake8.pycqa.org/en/latest/#), [Black](https://github.com/psf/black), and the rest of the hooks we have in our [pre-commit](https://pre-commit.com/) [config](https://github.com/dbt-labs/dbt-core/blob/75201be9db1cb2c6c01fa7e71a314f5e5beb060a/.pre-commit-config.yaml).
5051

5152
In some cases, the right resolution to an open issue might be tangential to the `dbt-core` codebase. The right path forward might be a documentation update or a change that can be made in user-space. In other cases, the issue might describe functionality that the `dbt-core` maintainers are unwilling or unable to incorporate into the `dbt-core` codebase. When it is determined that an open issue describes functionality that will not translate to a code change in the `dbt-core` repository, the issue will be tagged with the `wontfix` label (see below) and closed.
5253

@@ -106,6 +107,7 @@ A short list of tools used in `dbt-core` testing that will be helpful to your un
106107
- [`pytest`](https://docs.pytest.org/en/latest/) to discover/run tests
107108
- [`make`](https://users.cs.duke.edu/~ola/courses/programming/Makefiles/Makefiles.html) - but don't worry too much, nobody _really_ understands how make works and our Makefile is super simple
108109
- [`flake8`](https://flake8.pycqa.org/en/latest/) for code linting
110+
- [`black`](https://github.com/psf/black) for code formatting
109111
- [`mypy`](https://mypy.readthedocs.io/en/stable/) for static type checking
110112
- [Github Actions](https://github.com/features/actions)
111113

@@ -190,19 +192,18 @@ make test
190192
# Runs postgres integration tests with py38 in "fail fast" mode.
191193
make integration
192194
```
193-
> These make targets assume you have a recent version of [`tox`](https://tox.readthedocs.io/en/latest/) installed locally,
195+
> These make targets assume you have a local install of a recent version of [`tox`](https://tox.readthedocs.io/en/latest/) for unit/integration testing and pre-commit for code quality checks,
194196
> unless you use choose a Docker container to run tests. Run `make help` for more info.
195197
196198
Check out the other targets in the Makefile to see other commonly used test
197199
suites.
198200

201+
#### `pre-commit`
202+
[`pre-commit`](https.pre-commit.com) takes care of running all code-checks for formatting and linting. Run `make dev` to install `pre-commit` in your local environment. Once this is done you can use any of the linter-based make targets as well as a git pre-commit hook that will ensure proper formatting and linting.
203+
199204
#### `tox`
200205

201-
[`tox`](https://tox.readthedocs.io/en/latest/) takes care of managing virtualenvs and install dependencies in order to run
202-
tests. You can also run tests in parallel, for example, you can run unit tests
203-
for Python 3.7, Python 3.8, Python 3.9, `flake8` checks, and `mypy` checks in
204-
parallel with `tox -p`. Also, you can run unit tests for specific python versions
205-
with `tox -e py37`. The configuration for these tests in located in `tox.ini`.
206+
[`tox`](https://tox.readthedocs.io/en/latest/) takes care of managing virtualenvs and install dependencies in order to run tests. You can also run tests in parallel, for example, you can run unit tests for Python 3.7, Python 3.8, and Python 3.9 checks in parallel with `tox -p`. Also, you can run unit tests for specific python versions with `tox -e py37`. The configuration for these tests in located in `tox.ini`.
206207

207208
#### `pytest`
208209

Makefile

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,43 @@ endif
88

99
.PHONY: dev
1010
dev: ## Installs dbt-* packages in develop mode along with development dependencies.
11-
pip install -r dev-requirements.txt -r editable-requirements.txt
11+
@\
12+
pip install -r dev-requirements.txt -r editable-requirements.txt && \
13+
pre-commit install
1214

1315
.PHONY: mypy
14-
mypy: .env ## Runs mypy for static type checking.
15-
$(DOCKER_CMD) tox -e mypy
16+
mypy: .env ## Runs mypy against staged changes for static type checking.
17+
@\
18+
$(DOCKER_CMD) pre-commit run --hook-stage manual mypy-check | grep -v "INFO"
1619

1720
.PHONY: flake8
18-
flake8: .env ## Runs flake8 to enforce style guide.
19-
$(DOCKER_CMD) tox -e flake8
21+
flake8: .env ## Runs flake8 against staged changes to enforce style guide.
22+
@\
23+
$(DOCKER_CMD) pre-commit run --hook-stage manual flake8-check | grep -v "INFO"
24+
25+
.PHONY: black
26+
black: .env ## Runs black against staged changes to enforce style guide.
27+
@\
28+
$(DOCKER_CMD) pre-commit run --hook-stage manual black-check -v | grep -v "INFO"
2029

2130
.PHONY: lint
22-
lint: .env ## Runs all code checks in parallel.
23-
$(DOCKER_CMD) tox -p -e flake8,mypy
31+
lint: .env ## Runs flake8 and mypy code checks against staged changes.
32+
@\
33+
$(DOCKER_CMD) pre-commit run flake8-check --hook-stage manual | grep -v "INFO"; \
34+
$(DOCKER_CMD) pre-commit run mypy-check --hook-stage manual | grep -v "INFO"
2435

2536
.PHONY: unit
2637
unit: .env ## Runs unit tests with py38.
38+
@\
2739
$(DOCKER_CMD) tox -e py38
2840

2941
.PHONY: test
30-
test: .env ## Runs unit tests with py38 and code checks in parallel.
31-
$(DOCKER_CMD) tox -p -e py38,flake8,mypy
42+
test: .env ## Runs unit tests with py38 and code checks against staged changes.
43+
@\
44+
$(DOCKER_CMD) tox -p -e py38; \
45+
$(DOCKER_CMD) pre-commit run black-check --hook-stage manual | grep -v "INFO"; \
46+
$(DOCKER_CMD) pre-commit run flake8-check --hook-stage manual | grep -v "INFO"; \
47+
$(DOCKER_CMD) pre-commit run mypy-check --hook-stage manual | grep -v "INFO"
3248

3349
.PHONY: integration
3450
integration: .env integration-postgres ## Alias for integration-postgres.
@@ -38,15 +54,18 @@ integration-fail-fast: .env integration-postgres-fail-fast ## Alias for integrat
3854

3955
.PHONY: integration-postgres
4056
integration-postgres: .env ## Runs postgres integration tests with py38.
57+
@\
4158
$(DOCKER_CMD) tox -e py38-postgres -- -nauto
4259

4360
.PHONY: integration-postgres-fail-fast
4461
integration-postgres-fail-fast: .env ## Runs postgres integration tests with py38 in "fail fast" mode.
62+
@\
4563
$(DOCKER_CMD) tox -e py38-postgres -- -x -nauto
4664

4765
.PHONY: setup-db
4866
setup-db: ## Setup Postgres database with docker-compose for system testing.
49-
docker-compose up -d database
67+
@\
68+
docker-compose up -d database && \
5069
PGHOST=localhost PGUSER=root PGPASSWORD=password PGDATABASE=postgres bash test/setup_db.sh
5170

5271
# This rule creates a file named .env that is used by docker-compose for passing
@@ -62,26 +81,29 @@ endif
6281

6382
.PHONY: clean
6483
clean: ## Resets development environment.
65-
rm -f .coverage
66-
rm -rf .eggs/
67-
rm -f .env
68-
rm -rf .tox/
69-
rm -rf build/
70-
rm -rf dbt.egg-info/
71-
rm -f dbt_project.yml
72-
rm -rf dist/
73-
rm -f htmlcov/*.{css,html,js,json,png}
74-
rm -rf logs/
75-
rm -rf target/
76-
find . -type f -name '*.pyc' -delete
77-
find . -type d -name '__pycache__' -depth -delete
84+
@echo 'cleaning repo...'
85+
@rm -f .coverage
86+
@rm -rf .eggs/
87+
@rm -f .env
88+
@rm -rf .tox/
89+
@rm -rf build/
90+
@rm -rf dbt.egg-info/
91+
@rm -f dbt_project.yml
92+
@rm -rf dist/
93+
@rm -f htmlcov/*.{css,html,js,json,png}
94+
@rm -rf logs/
95+
@rm -rf target/
96+
@find . -type f -name '*.pyc' -delete
97+
@find . -type d -name '__pycache__' -depth -delete
98+
@echo 'done.'
99+
78100

79101
.PHONY: help
80102
help: ## Show this help message.
81103
@echo 'usage: make [target] [USE_DOCKER=true]'
82104
@echo
83105
@echo 'targets:'
84-
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
106+
@grep -E '^[8+a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
85107
@echo
86108
@echo 'options:'
87109
@echo 'use USE_DOCKER=true to run target in a docker container'

dev-requirements.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
black==21.12b0
12
bumpversion
23
flake8
34
flaky
45
freezegun==0.3.12
56
ipdb
67
mypy==0.782
78
pip-tools
9+
pre-commit
810
pytest
911
pytest-dotenv
1012
pytest-logbook

tox.ini

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,6 @@
11
[tox]
22
skipsdist = True
3-
envlist = py37,py38,py39,flake8,mypy
4-
5-
[testenv:flake8]
6-
description = flake8 code checks
7-
basepython = python3.8
8-
skip_install = true
9-
commands = flake8 --select=E,W,F --ignore=W503,W504,E741,E203 --max-line-length 99 \
10-
core/dbt \
11-
plugins/postgres/dbt
12-
deps =
13-
-rdev-requirements.txt
14-
15-
[testenv:mypy]
16-
description = mypy static type checking
17-
basepython = python3.8
18-
skip_install = true
19-
commands = mypy --show-error-codes core/dbt
20-
deps =
21-
-rdev-requirements.txt
22-
-reditable-requirements.txt
3+
envlist = py37,py38,py39
234

245
[testenv:{unit,py37,py38,py39,py}]
256
description = unit testing

0 commit comments

Comments
 (0)