Skip to content

Commit

Permalink
Move to pre-commit only (#1026)
Browse files Browse the repository at this point in the history
* move linter and typechecker reqs and config to .pre-commit-config.yaml
* update linters and typecheckers
* make updates from running linters and typecheckers
* remove old make recipes
  • Loading branch information
mikealfare authored May 8, 2024
1 parent b657767 commit 94bfcd9
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 130 deletions.
14 changes: 0 additions & 14 deletions .flake8

This file was deleted.

1 change: 0 additions & 1 deletion .github/workflows/integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ on:
pull_request_target:
paths-ignore:
- ".changes/**"
- ".flake8"
- ".gitignore"
- "**.md"

Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ jobs:
python -m pip --version
python -m pip install pre-commit
pre-commit --version
python -m pip install mypy==0.942
python -m pip install types-requests
mypy --version
python -m pip install -r requirements.txt
python -m pip install -r dev-requirements.txt
python -c "import dbt.adapters.spark"
Expand Down
115 changes: 53 additions & 62 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,67 +1,58 @@
# For more on configuring pre-commit hooks (see https://pre-commit.com/)

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

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.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/dbt-labs/pre-commit-hooks
rev: v0.1.0a1
hooks:
- id: dbt-core-in-adapters-check
- repo: https://github.com/psf/black
rev: 23.1.0
hooks:
- id: black
additional_dependencies: ['click~=8.1']
args:
- "--line-length=99"
- "--target-version=py38"
- id: black
alias: black-check
stages: [manual]
additional_dependencies: ['click~=8.1']
args:
- "--line-length=99"
- "--target-version=py38"
- "--check"
- "--diff"
- repo: https://github.com/pycqa/flake8
rev: 6.0.0
hooks:
- id: flake8
- id: flake8
alias: flake8-check
stages: [manual]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.2.0
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.
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.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/dbt-labs/pre-commit-hooks
rev: v0.1.0a1
hooks:
- id: dbt-core-in-adapters-check

- repo: https://github.com/psf/black
rev: 24.4.2
hooks:
- id: black
args:
- --line-length=99
- --target-version=py38
- --target-version=py39
- --target-version=py310
- --target-version=py311
additional_dependencies: [flaky]

- repo: https://github.com/pycqa/flake8
rev: 7.0.0
hooks:
- id: flake8
exclude: tests/
args:
- --max-line-length=99
- --select=E,F,W
- --ignore=E203,E501,E741,W503,W504
- --per-file-ignores=*/__init__.py:F401

# 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, --explicit-package-bases, --warn-unused-ignores, --disallow-untyped-defs]
files: ^dbt/adapters/.*
language: system
- id: mypy
alias: mypy-check
stages: [manual]
args: [--show-error-codes, --pretty, --ignore-missing-imports, --explicit-package-bases]
files: ^dbt/adapters
language: system
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.10.0
hooks:
- id: mypy
args:
- --show-error-codes
- --ignore-missing-imports
- --explicit-package-bases
- --warn-unused-ignores
- --disallow-untyped-defs
- --pretty
files: ^dbt/adapters
additional_dependencies:
- types-pytz
- types-requests
27 changes: 2 additions & 25 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,10 @@ dev-uninstall: ## Uninstalls all packages while maintaining the virtual environm
pip freeze | grep -v "^-e" | cut -d "@" -f1 | xargs pip uninstall -y
pip uninstall -y dbt-spark

.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: linecheck
linecheck: ## Checks for all Python lines 100 characters or more
@\
find dbt -type f -name "*.py" -exec grep -I -r -n '.\{100\}' {} \;
pre-commit run --all-files

.PHONY: unit
unit: ## Runs unit tests with py38.
Expand All @@ -47,9 +26,7 @@ test: ## Runs unit tests with py38 and code checks against staged changes.
@\
python -m pytest tests/unit; \
python dagger/run_dbt_spark_tests.py --profile spark_session \
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"
pre-commit run --all-files

.PHONY: clean
@echo "cleaning repo"
Expand Down
2 changes: 1 addition & 1 deletion dbt/adapters/spark/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
from dbt.include import spark

Plugin = AdapterPlugin(
adapter=SparkAdapter, credentials=SparkCredentials, include_path=spark.PACKAGE_PATH # type: ignore
adapter=SparkAdapter, credentials=SparkCredentials, include_path=spark.PACKAGE_PATH
)
2 changes: 1 addition & 1 deletion dbt/adapters/spark/column.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SparkColumn(dbtClassMixin, Column):
def translate_type(cls, dtype: str) -> str:
return dtype

def can_expand_to(self: Self, other_column: Self) -> bool: # type: ignore
def can_expand_to(self: Self, other_column: Self) -> bool:
"""returns True if both columns are strings"""
return self.is_string() and other_column.is_string()

Expand Down
6 changes: 3 additions & 3 deletions dbt/adapters/spark/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ class SparkConnectionMethod(StrEnum):
@dataclass
class SparkCredentials(Credentials):
host: Optional[str] = None
schema: Optional[str] = None # type: ignore
schema: Optional[str] = None
method: SparkConnectionMethod = None # type: ignore
database: Optional[str] = None # type: ignore
database: Optional[str] = None
driver: Optional[str] = None
cluster: Optional[str] = None
endpoint: Optional[str] = None
Expand Down Expand Up @@ -568,7 +568,7 @@ def open(cls, connection: Connection) -> Connection:
return connection

@classmethod
def data_type_code_to_name(cls, type_code: Union[type, str]) -> str: # type: ignore
def data_type_code_to_name(cls, type_code: Union[type, str]) -> str:
"""
:param Union[type, str] type_code: The sql to execute.
* type_code is a python type (!) in pyodbc https://github.com/mkleehammer/pyodbc/wiki/Cursor#description, and a string for other spark runtimes.
Expand Down
2 changes: 1 addition & 1 deletion dbt/adapters/spark/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def convert_time_type(cls, agate_table: agate.Table, col_idx: int) -> str:
def convert_datetime_type(cls, agate_table: agate.Table, col_idx: int) -> str:
return "timestamp"

def quote(self, identifier: str) -> str: # type: ignore
def quote(self, identifier: str) -> str:
return "`{}`".format(identifier)

def _get_relation_information(self, row: agate.Row) -> RelationInfo:
Expand Down
30 changes: 11 additions & 19 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,22 @@ git+https://github.com/dbt-labs/dbt-common.git
git+https://github.com/dbt-labs/dbt-adapters.git
git+https://github.com/dbt-labs/dbt-adapters.git#subdirectory=dbt-tests-adapter

# if version 1.x or greater -> pin to major version
# if version 0.x -> pin to minor
black>=24.3
bumpversion~=0.6.0
click~=8.1
flake8~=6.1;python_version>="3.8"
flaky~=3.7
freezegun~=1.3
# dev
ipdb~=0.13.13
mypy==1.7.1 # patch updates have historically introduced breaking changes
pip-tools~=7.3
pre-commit~=3.5
pre-commit-hooks~=4.5
pre-commit==3.7.0;python_version >="3.9"
pre-commit==3.5.0;python_version <"3.9"

# test
freezegun~=1.3
mock~=5.1
pytest~=7.4
pytest-csv~=3.0
pytest-dotenv~=0.5.2
pytest-logbook~=1.2
pytest-xdist~=3.5
pytz~=2023.3
types-pytz~=2023.3
types-requests~=2.31
thrift_sasl~=0.4.3

# build
bumpversion~=0.6.0
twine~=4.0
wheel~=0.42

# Adapter specific dependencies
mock~=5.1
thrift_sasl~=0.4.3
1 change: 1 addition & 0 deletions tests/unit/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Note that all imports should be inside the functions to avoid import/mocking
issues.
"""

import string
import os
from unittest import mock
Expand Down

0 comments on commit 94bfcd9

Please sign in to comment.