Move to pre-commit only (#996)

* move all linting and typing config into .pre-commit-config.yaml * update pre-commit packages * run linters and make updates * remove make recipes that are no longer relevant * add unused type ignore comment check
dbt-labs · May 8, 2024 · 5b92881 · 5b92881
1 parent 1224b0e
commit 5b92881
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 157 deletions.
diff --git a/.flake8 b/.flake8
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -58,7 +58,6 @@ jobs:
           python -m pip install -r dev-requirements.txt
           python -m pip --version
           pre-commit --version
-          mypy --version
           dbt --version
 
       - name: Run pre-commit hooks

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,67 +1,57 @@
 # 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.1.1
-  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]
-    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
+        -   --pretty
+        files: ^dbt/adapters
+        additional_dependencies:
+        -   types-pytz
+        -   types-requests
diff --git a/Makefile b/Makefile
@@ -11,32 +11,6 @@ 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-snowflake
 
-.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\}' {} \;
-
 .PHONY: unit
 unit: ## Runs unit tests with py38.
 	@\
@@ -46,9 +20,7 @@ unit: ## Runs unit tests with py38.
 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"
+	pre-commit run --all-files
 
 .PHONY: integration
 integration: ## Runs snowflake integration tests with py38.

diff --git a/dbt/adapters/snowflake/__init__.py b/dbt/adapters/snowflake/__init__.py
@@ -1,12 +1,12 @@
-from dbt.adapters.snowflake.column import SnowflakeColumn  # noqa
-from dbt.adapters.snowflake.connections import SnowflakeConnectionManager  # noqa
+from dbt.adapters.snowflake.column import SnowflakeColumn
+from dbt.adapters.snowflake.connections import SnowflakeConnectionManager
 from dbt.adapters.snowflake.connections import SnowflakeCredentials
-from dbt.adapters.snowflake.relation import SnowflakeRelation  # noqa
+from dbt.adapters.snowflake.relation import SnowflakeRelation
 from dbt.adapters.snowflake.impl import SnowflakeAdapter
 
-from dbt.adapters.base import AdapterPlugin  # type: ignore
-from dbt.include import snowflake  # type: ignore
+from dbt.adapters.base import AdapterPlugin
+from dbt.include import snowflake
 
 Plugin = AdapterPlugin(
-    adapter=SnowflakeAdapter, credentials=SnowflakeCredentials, include_path=snowflake.PACKAGE_PATH  # type: ignore
+    adapter=SnowflakeAdapter, credentials=SnowflakeCredentials, include_path=snowflake.PACKAGE_PATH
 )
diff --git a/dbt/adapters/snowflake/connections.py b/dbt/adapters/snowflake/connections.py
@@ -40,8 +40,8 @@
 from dbt_common.exceptions import DbtDatabaseError
 from dbt.adapters.exceptions.connection import FailedToConnectError
 from dbt.adapters.contracts.connection import AdapterResponse, Connection, Credentials
-from dbt.adapters.sql import SQLConnectionManager  # type: ignore
-from dbt.adapters.events.logging import AdapterLogger  # type: ignore
+from dbt.adapters.sql import SQLConnectionManager
+from dbt.adapters.events.logging import AdapterLogger
 from dbt_common.events.functions import warn_or_error
 from dbt.adapters.events.types import AdapterEventWarning
 from dbt_common.ui import line_wrap_message, warning_tag
@@ -415,7 +415,7 @@ def get_response(cls, cursor) -> SnowflakeAdapterResponse:
             rows_affected=cursor.rowcount,
             code=code,
             query_id=cursor.sfqid,
-        )  # type: ignore
+        )
 
     # disable transactional logic by default on Snowflake
     # except for DML statements where explicitly defined
@@ -487,7 +487,7 @@ def add_query(
         auto_begin: bool = True,
         bindings: Optional[Any] = None,
         abridge_sql_log: bool = False,
-    ) -> Tuple[Connection, Any]:  # type: ignore
+    ) -> Tuple[Connection, Any]:
         if bindings:
             # The snowflake connector is stricter than, e.g., psycopg2 -
             # which allows any iterable thing to be passed as a binding.
@@ -513,7 +513,7 @@ def add_query(
         if cursor is None:
             self._raise_cursor_not_found_error(sql)
 
-        return connection, cursor  # type: ignore
+        return connection, cursor
 
     def _stripped_queries(self, sql: str) -> List[str]:
         def strip_query(query):
@@ -580,7 +580,7 @@ def release(self):
         """Reuse connections by deferring release until adapter context manager in core
         resets adapters. This cleanup_all happens before Python teardown. Idle connections
         incur no costs while waiting in the connection pool."""
-        if self.profile.credentials.reuse_connections:  # type: ignore
+        if self.profile.credentials.reuse_connections:
             return
         super().release()
 

diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py
@@ -3,10 +3,10 @@
 
 import agate
 
-from dbt.adapters.base.impl import AdapterConfig, ConstraintSupport  # type: ignore
+from dbt.adapters.base.impl import AdapterConfig, ConstraintSupport
 from dbt.adapters.base.meta import available
 from dbt.adapters.capability import CapabilityDict, CapabilitySupport, Support, Capability
-from dbt.adapters.sql import SQLAdapter  # type: ignore
+from dbt.adapters.sql import SQLAdapter
 from dbt.adapters.sql.impl import (
     LIST_SCHEMAS_MACRO_NAME,
     LIST_RELATIONS_MACRO_NAME,
@@ -129,7 +129,9 @@ def get_columns_in_relation(self, relation):
             else:
                 raise
 
-    def list_relations_without_caching(self, schema_relation: SnowflakeRelation) -> List[SnowflakeRelation]:  # type: ignore
+    def list_relations_without_caching(
+        self, schema_relation: SnowflakeRelation
+    ) -> List[SnowflakeRelation]:
         kwargs = {"schema_relation": schema_relation}
         try:
             results = self.execute_macro(LIST_RELATIONS_MACRO_NAME, kwargs=kwargs)
@@ -145,7 +147,7 @@ def list_relations_without_caching(self, schema_relation: SnowflakeRelation) ->
         quote_policy = {"database": True, "schema": True, "identifier": True}
 
         columns = ["database_name", "schema_name", "name", "kind"]
-        for _database, _schema, _identifier, _type in results.select(columns):  # type: ignore
+        for _database, _schema, _identifier, _type in results.select(columns):
             try:
                 _type = self.Relation.get_relation_type(_type.lower())
             except ValueError:

diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py
@@ -18,24 +18,24 @@
 
 @dataclass(frozen=True, eq=False, repr=False)
 class SnowflakeRelation(BaseRelation):
-    type: Optional[SnowflakeRelationType] = None  # type: ignore
+    type: Optional[SnowflakeRelationType] = None
     quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy())
     require_alias: bool = False
     renameable_relations: FrozenSet[SnowflakeRelationType] = field(
         default_factory=lambda: frozenset(
             {
-                SnowflakeRelationType.Table,
-                SnowflakeRelationType.View,
+                SnowflakeRelationType.Table,  # type: ignore
+                SnowflakeRelationType.View,  # type: ignore
             }
         )
     )
 
     replaceable_relations: FrozenSet[SnowflakeRelationType] = field(
         default_factory=lambda: frozenset(
             {
-                SnowflakeRelationType.DynamicTable,
-                SnowflakeRelationType.Table,
-                SnowflakeRelationType.View,
+                SnowflakeRelationType.DynamicTable,  # type: ignore
+                SnowflakeRelationType.Table,  # type: ignore
+                SnowflakeRelationType.View,  # type: ignore
             }
         )
     )

diff --git a/dbt/adapters/snowflake/relation_configs/base.py b/dbt/adapters/snowflake/relation_configs/base.py
@@ -45,7 +45,7 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict:
     def from_relation_results(cls, relation_results: RelationResults):
         relation_config = cls.parse_relation_results(relation_results)
         relation = cls.from_dict(relation_config)
-        return relation  # type: ignore
+        return relation
 
     @classmethod
     def parse_relation_results(cls, relation_results: RelationResults) -> Dict[str, Any]:

diff --git a/dbt/adapters/snowflake/relation_configs/dynamic_table.py b/dbt/adapters/snowflake/relation_configs/dynamic_table.py
@@ -44,7 +44,7 @@ def from_dict(cls, config_dict) -> "SnowflakeDynamicTableConfig":
             "snowflake_warehouse": config_dict.get("snowflake_warehouse"),
         }
 
-        dynamic_table: "SnowflakeDynamicTableConfig" = super().from_dict(kwargs_dict)  # type: ignore
+        dynamic_table: "SnowflakeDynamicTableConfig" = super().from_dict(kwargs_dict)
         return dynamic_table
 
     @classmethod
@@ -53,9 +53,9 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict[str, Any
             "name": relation_config.identifier,
             "schema_name": relation_config.schema,
             "database_name": relation_config.database,
-            "query": relation_config.compiled_code,  # type: ignore
-            "target_lag": relation_config.config.extra.get("target_lag"),  # type: ignore
-            "snowflake_warehouse": relation_config.config.extra.get("snowflake_warehouse"),  # type: ignore
+            "query": relation_config.compiled_code,
+            "target_lag": relation_config.config.extra.get("target_lag"),
+            "snowflake_warehouse": relation_config.config.extra.get("snowflake_warehouse"),
         }
 
         return config_dict
@@ -104,9 +104,11 @@ def requires_full_refresh(self) -> bool:
         return any(
             [
                 self.target_lag.requires_full_refresh if self.target_lag else False,
-                self.snowflake_warehouse.requires_full_refresh
-                if self.snowflake_warehouse
-                else False,
+                (
+                    self.snowflake_warehouse.requires_full_refresh
+                    if self.snowflake_warehouse
+                    else False
+                ),
             ]
         )