Skip to content
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

Ct 177/add precommit hooks #107

Merged
merged 9 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
12 changes: 12 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[flake8]
select =
E
W
F
ignore =
W503 # makes Flake8 work like black
W504
E203 # makes Flake8 work like black
E741
max-line-length = 99
VersusFacit marked this conversation as resolved.
Show resolved Hide resolved
exclude = test
23 changes: 9 additions & 14 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,10 @@ defaults:

jobs:
code-quality:
name: ${{ matrix.toxenv }}
name: code-quality

runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
toxenv: [flake8]

env:
TOXENV: ${{ matrix.toxenv }}
PYTEST_ADDOPTS: "-v --color=yes"

steps:
- name: Check out the repository
uses: actions/checkout@v2
Expand All @@ -62,12 +53,16 @@ jobs:
- name: Install python dependencies
run: |
pip install --user --upgrade pip
pip install tox
pip install pre-commit
pip install mypy==0.782
pip install -r dev_requirements.txt
pip --version
tox --version
pre-commit --version
mypy --version
dbt --version

- name: Run tox
run: tox
- name: Run pre-commit hooks
run: pre-commit run --all-files --show-diff-on-failure

unit:
name: unit test / python ${{ matrix.python-version }}
Expand Down
10 changes: 5 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ coverage.xml
*,cover
.hypothesis/
test.env

# Mypy
.mypy_cache/
*.pytest_cache/

# Translations
*.mo
Expand All @@ -66,10 +64,10 @@ docs/_build/
# PyBuilder
target/

#Ipython Notebook
# Ipython Notebook
.ipynb_checkpoints

#Emacs
# Emacs
*~

# Sublime Text
Expand All @@ -78,6 +76,7 @@ target/
# Vim
*.sw*

# Pyenv
.python-version

# Vim
Expand All @@ -90,6 +89,7 @@ venv/
# AWS credentials
.aws/

# MacOS
.DS_Store

# vscode
Expand Down
64 changes: 64 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# For more on configuring pre-commit hooks (see https://pre-commit.com/)

# TODO: remove global exclusion of tests when testing overhaul is complete
exclude: '^tests/.*'

# Force all unspecified python hooks to run python 3.8
default_language_version:
python: python3.8

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: check-yaml
args: [--unsafe]
- id: check-json
- id: end-of-file-fixer
- id: trailing-whitespace
- id: check-case-conflict
- repo: https://github.com/psf/black
rev: 21.12b0
hooks:
- id: black
args:
- "--line-length=99"
- "--target-version=py38"
- id: black
alias: black-check
stages: [manual]
args:
- "--line-length=99"
- "--target-version=py38"
- "--check"
- "--diff"
- repo: https://gitlab.com/pycqa/flake8
rev: 4.0.1
hooks:
- id: flake8
- id: flake8
alias: flake8-check
stages: [manual]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.782
hooks:
- id: mypy
# N.B.: Mypy is... a bit fragile.
#
# By using `language: system` we run this hook in the local
# environment instead of a pre-commit isolated one. This is needed
# to ensure mypy correctly parses the project.

# It may cause trouble in that it adds environmental variables out
# of our control to the mix. Unfortunately, there's nothing we can
# do about per pre-commit's author.
# See https://github.com/pre-commit/pre-commit/issues/730 for details.
args: [--show-error-codes, --ignore-missing-imports]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the --ignore-missing-imports flag here to reduce in-code clutter. My argument is that all these errors tell you is that the upstream project lacks static type information, which isn't that powerful of knowledge.

files: ^dbt/adapters/.*
language: system
- id: mypy
alias: mypy-check
stages: [manual]
args: [--show-error-codes, --pretty, --ignore-missing-imports]
files: ^dbt/adapters
language: system
58 changes: 58 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
.DEFAULT_GOAL:=help

.PHONY: dev
dev: ## Installs adapter in develop mode along with development depedencies
@\
pip install -r dev_requirements.txt && pre-commit install

.PHONY: mypy
mypy: ## Runs mypy against staged changes for static type checking.
@\
pre-commit run --hook-stage manual mypy-check | grep -v "INFO"

.PHONY: flake8
flake8: ## Runs flake8 against staged changes to enforce style guide.
@\
pre-commit run --hook-stage manual flake8-check | grep -v "INFO"

.PHONY: black
black: ## Runs black against staged changes to enforce style guide.
@\
pre-commit run --hook-stage manual black-check -v | grep -v "INFO"


.PHONY: lint
lint: ## Runs flake8 and mypy code checks against staged changes.
@\
pre-commit run flake8-check --hook-stage manual | grep -v "INFO"; \
pre-commit run mypy-check --hook-stage manual | grep -v "INFO"


.PHONY: unit
unit: ## Runs unit tests with py38.
@\
tox -e py38

.PHONY: test
test: ## Runs unit tests with py38 and code checks against staged changes.
@\
tox -p -e py38; \
pre-commit run black-check --hook-stage manual | grep -v "INFO"; \
pre-commit run flake8-check --hook-stage manual | grep -v "INFO"; \
pre-commit run mypy-check --hook-stage manual | grep -v "INFO"

.PHONY: integration
integration: ## Runs snowflake integration tests with py38.
@\
tox -e py38-snowflake --

.PHONY: clean
@echo "cleaning repo"
@git clean -f -X

.PHONY: help
help: ## Show this help message.
@echo 'usage: make [target]'
@echo
@echo 'targets:'
@grep -E '^[7+a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
4 changes: 2 additions & 2 deletions dbt/adapters/snowflake/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
from dbt.adapters.snowflake.relation import SnowflakeRelation # noqa
from dbt.adapters.snowflake.impl import SnowflakeAdapter

from dbt.adapters.base import AdapterPlugin
from dbt.include import snowflake
from dbt.adapters.base import AdapterPlugin # type: ignore
from dbt.include import snowflake # type: ignore

Plugin = AdapterPlugin(
adapter=SnowflakeAdapter,
Expand Down
6 changes: 3 additions & 3 deletions dbt/adapters/snowflake/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
InternalException, RuntimeException, FailedToConnectException,
DatabaseException, warn_or_error
)
from dbt.adapters.base import Credentials
from dbt.adapters.base import Credentials # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

attr-defined errors are masked by this # type: ignore declaration

from dbt.contracts.connection import AdapterResponse
from dbt.adapters.sql import SQLConnectionManager
from dbt.events import AdapterLogger
from dbt.adapters.sql import SQLConnectionManager # type: ignore
from dbt.events import AdapterLogger # type: ignore


logger = AdapterLogger("Snowflake")
Expand Down
2 changes: 1 addition & 1 deletion dbt/adapters/snowflake/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import agate

from dbt.adapters.base.impl import AdapterConfig
from dbt.adapters.sql import SQLAdapter
from dbt.adapters.sql import SQLAdapter # type: ignore
from dbt.adapters.sql.impl import (
LIST_SCHEMAS_MACRO_NAME,
LIST_RELATIONS_MACRO_NAME,
Expand Down
2 changes: 2 additions & 0 deletions dev_requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
# TODO: how to automate switching from develop to version branches?
git+https://github.com/dbt-labs/dbt.git#egg=dbt-core&subdirectory=core

black==21.12b0
bumpversion
flake8
flaky
freezegun==0.3.12
ipdb
mypy==0.782
pip-tools
pre-commit
pytest
pytest-dotenv
pytest-logbook
Expand Down
11 changes: 1 addition & 10 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
[tox]
skipsdist = True
envlist = py37,py38,py39,flake8

[testenv:flake8]
description = flake8 code checks
basepython = python3.8
skip_install = true
commands = flake8 --select=E,W,F --ignore=W504,E741 --max-line-length 99 \
dbt
deps =
-rdev_requirements.txt
envlist = py37,py38,py39

[testenv:{unit,py37,py38,py39,py}]
description = unit testing
Expand Down