Move code quality deps to precommit (#1291)

* move linter config into .pre-commit-config.yaml
* make updates from mypy - all unneeded type ignore comments
* update contributing guide to better reflect current state

Author: mikealfare

.changes/unreleased/Under the Hood-20240718-193206.yaml

@@ -0,0 +1,6 @@
+kind: Under the Hood
+body: Simplify linting environment and dev dependencies
+time: 2024-07-18T19:32:06.044016-04:00
+custom:
+  Author: mikealfare
+  Issue: "1291"

.flake8

@@ -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-comit hooks
         run: pre-commit run --all-files --show-diff-on-failure

.github/workflows/main.yml

@@ -1,66 +1,58 @@
 # For more on configuring pre-commit hooks (see https://pre-commit.com/)
-
 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:
+        -   --explicit-package-bases
+        -   --ignore-missing-imports
+        -   --pretty
+        -   --show-error-codes
+        -   --warn-unused-ignores
+        files: ^dbt/adapters/bigquery
+        additional_dependencies:
+        -   types-protobuf
+        -   types-pytz
+        -   types-requests

.pre-commit-config.yaml

@@ -67,7 +67,7 @@ $EDITOR test.env
 There are a few methods for running tests locally.
 
 #### `tox`
-`tox` takes care of managing Python virtualenvs and installing dependencies in order to run tests. You can also run tests in parallel, for example you can run unit tests for Python 3.8, Python 3.9, and `flake8` checks in parallel with `tox -p`. Also, you can run unit tests for specific python versions with `tox -e py38`. The configuration of these tests are located in `tox.ini`.
+`tox` takes care of managing Python virtualenvs and installing dependencies in order to run tests. You can also run tests in parallel, for example you can run unit tests for Python 3.8, Python 3.9, Python 3.10, and Python 3.11 in parallel with `tox -p`. Also, you can run unit tests for specific python versions with `tox -e py38`. The configuration of these tests are located in `tox.ini`.
 
 #### `pytest`
 Finally, you can also run a specific test or group of tests using `pytest` directly. With a Python virtualenv active and dev dependencies installed you can do things like:

CONTRIBUTING.md

@@ -8,5 +8,5 @@
 from dbt.include import bigquery
 
 Plugin = AdapterPlugin(
-    adapter=BigQueryAdapter, credentials=BigQueryCredentials, include_path=bigquery.PACKAGE_PATH  # type: ignore[arg-type]
+    adapter=BigQueryAdapter, credentials=BigQueryCredentials, include_path=bigquery.PACKAGE_PATH
 )

dbt/adapters/bigquery/__init__.py

@@ -18,7 +18,7 @@ class BigQueryColumn(Column):
         "INTEGER": "INT64",
     }
     fields: List[Self]  # type: ignore
-    mode: str  # type: ignore
+    mode: str
 
     def __init__(
         self,
@@ -110,7 +110,7 @@ def is_numeric(self) -> bool:
     def is_float(self):
         return self.dtype.lower() == "float64"
 
-    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()
 
@@ -124,7 +124,7 @@ def column_to_bq_schema(self) -> SchemaField:
             fields = [field.column_to_bq_schema() for field in self.fields]  # type: ignore[attr-defined]
             kwargs = {"fields": fields}
 
-        return SchemaField(self.name, self.dtype, self.mode, **kwargs)  # type: ignore[arg-type]
+        return SchemaField(self.name, self.dtype, self.mode, **kwargs)
 
 
 def get_nested_column_data_types(

dbt/adapters/bigquery/column.py

@@ -116,8 +116,8 @@ class BigQueryCredentials(Credentials):
 
     # BigQuery allows an empty database / project, where it defers to the
     # environment for the project
-    database: Optional[str] = None  # type: ignore
-    schema: Optional[str] = None  # type: ignore
+    database: Optional[str] = None
+    schema: Optional[str] = None
     execution_project: Optional[str] = None
     location: Optional[str] = None
     priority: Optional[Priority] = None
@@ -568,7 +568,7 @@ def execute(
         else:
             message = f"{code}"
 
-        response = BigQueryAdapterResponse(  # type: ignore[call-arg]
+        response = BigQueryAdapterResponse(
             _message=message,
             rows_affected=num_rows,
             code=code,

dbt/adapters/bigquery/connections.py

@@ -20,9 +20,9 @@ def create_batch_request(
     batch: Batch, batch_id: str, project: str, region: str
 ) -> CreateBatchRequest:
     return CreateBatchRequest(
-        parent=f"projects/{project}/locations/{region}",  # type: ignore
-        batch_id=batch_id,  # type: ignore
-        batch=batch,  # type: ignore
+        parent=f"projects/{project}/locations/{region}",
+        batch_id=batch_id,
+        batch=batch,
     )
 
 
@@ -35,10 +35,10 @@ def poll_batch_job(
     run_time = 0
     while state in _BATCH_RUNNING_STATES and run_time < timeout:
         time.sleep(1)
-        response = job_client.get_batch(  # type: ignore
-            request=GetBatchRequest(name=batch_name),  # type: ignore
+        response = job_client.get_batch(
+            request=GetBatchRequest(name=batch_name),
         )
-        run_time = datetime.now().timestamp() - response.create_time.timestamp()  # type: ignore
+        run_time = datetime.now().timestamp() - response.create_time.timestamp()
         state = response.state
     if not response:
         raise ValueError("No response from Dataproc")

dbt/adapters/bigquery/dataproc/batch.py

@@ -22,7 +22,7 @@
 from dbt.adapters.contracts.relation import RelationConfig
 
 import dbt_common.exceptions.base
-from dbt.adapters.base import (  # type: ignore
+from dbt.adapters.base import (
     AdapterConfig,
     BaseAdapter,
     BaseRelation,
@@ -33,11 +33,15 @@
     available,
 )
 from dbt.adapters.base.impl import FreshnessResponse
-from dbt.adapters.cache import _make_ref_key_dict  # type: ignore
+from dbt.adapters.cache import _make_ref_key_dict
 from dbt.adapters.capability import Capability, CapabilityDict, CapabilitySupport, Support
 from dbt.adapters.contracts.connection import AdapterResponse
 from dbt.adapters.contracts.macros import MacroResolverProtocol
-from dbt_common.contracts.constraints import ColumnLevelConstraint, ConstraintType, ModelLevelConstraint  # type: ignore
+from dbt_common.contracts.constraints import (
+    ColumnLevelConstraint,
+    ConstraintType,
+    ModelLevelConstraint,
+)
 from dbt_common.dataclass_schema import dbtClassMixin
 from dbt.adapters.events.logging import AdapterLogger
 from dbt_common.events.functions import fire_event
@@ -163,7 +167,7 @@ def is_cancelable(cls) -> bool:
         return False
 
     def drop_relation(self, relation: BigQueryRelation) -> None:
-        is_cached = self._schema_is_cached(relation.database, relation.schema)  # type: ignore[arg-type]
+        is_cached = self._schema_is_cached(relation.database, relation.schema)
         if is_cached:
             self.cache_dropped(relation)
 
@@ -258,7 +262,7 @@ def add_time_ingestion_partition_column(self, partition_by, columns) -> List[Big
         )
         return columns
 
-    def expand_column_types(self, goal: BigQueryRelation, current: BigQueryRelation) -> None:  # type: ignore[override]
+    def expand_column_types(self, goal: BigQueryRelation, current: BigQueryRelation) -> None:
         # This is a no-op on BigQuery
         pass
 
@@ -323,7 +327,7 @@ def get_relation(
     # TODO: the code below is copy-pasted from SQLAdapter.create_schema. Is there a better way?
     def create_schema(self, relation: BigQueryRelation) -> None:
         # use SQL 'create schema'
-        relation = relation.without_identifier()  # type: ignore
+        relation = relation.without_identifier()
 
         fire_event(SchemaCreation(relation=_make_ref_key_dict(relation)))
         kwargs = {
@@ -410,7 +414,7 @@ def _agate_to_schema(
         for idx, col_name in enumerate(agate_table.column_names):
             inferred_type = self.convert_agate_type(agate_table, idx)
             type_ = column_override.get(col_name, inferred_type)
-            bq_schema.append(SchemaField(col_name, type_))  # type: ignore[arg-type]
+            bq_schema.append(SchemaField(col_name, type_))
         return bq_schema
 
     @available.parse(lambda *a, **k: "")
@@ -736,8 +740,8 @@ def _get_catalog_schemas(self, relation_config: Iterable[RelationConfig]) -> Sch
         for candidate, schemas in candidates.items():
             database = candidate.database
             if database not in db_schemas:
-                db_schemas[database] = set(self.list_schemas(database))  # type: ignore[index]
-            if candidate.schema in db_schemas[database]:  # type: ignore[index]
+                db_schemas[database] = set(self.list_schemas(database))
+            if candidate.schema in db_schemas[database]:
                 result[candidate] = schemas
             else:
                 logger.debug(
@@ -844,7 +848,7 @@ def describe_relation(
         return None
 
     @available.parse_none
-    def grant_access_to(self, entity, entity_type, role, grant_target_dict):
+    def grant_access_to(self, entity, entity_type, role, grant_target_dict) -> None:
         """
         Given an entity, grants it access to a dataset.
         """
@@ -873,7 +877,7 @@ def get_dataset_location(self, relation):
         dataset = client.get_dataset(dataset_ref)
         return dataset.location
 
-    def get_rows_different_sql(  # type: ignore[override]
+    def get_rows_different_sql(
         self,
         relation_a: BigQueryRelation,
         relation_b: BigQueryRelation,
@@ -921,7 +925,7 @@ def run_sql_for_tests(self, sql, fetch, conn=None):
             return list(res)
 
     def generate_python_submission_response(self, submission_result) -> BigQueryAdapterResponse:
-        return BigQueryAdapterResponse(_message="OK")  # type: ignore[call-arg]
+        return BigQueryAdapterResponse(_message="OK")
 
     @property
     def default_python_submission_method(self) -> str:
@@ -961,7 +965,7 @@ def render_raw_columns_constraints(cls, raw_columns: Dict[str, Dict[str, Any]])
 
     @classmethod
     def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional[str]:
-        c = super().render_column_constraint(constraint)  # type: ignore
+        c = super().render_column_constraint(constraint)
         if (
             constraint.type == ConstraintType.primary_key
             or constraint.type == ConstraintType.foreign_key
@@ -971,7 +975,7 @@ def render_column_constraint(cls, constraint: ColumnLevelConstraint) -> Optional
 
     @classmethod
     def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[str]:
-        c = super().render_model_constraint(constraint)  # type: ignore
+        c = super().render_model_constraint(constraint)
         if (
             constraint.type == ConstraintType.primary_key
             or constraint.type == ConstraintType.foreign_key