Add isort and a corresponding CI check (#533)

* Sort `dev-requirements` alphabetically

* Add `isort`, configuration, `make` target

* Execute `isort` to sort unsorted imports

* Update `CHANGELOG.md`

* Differentiate `make isort` and `make isort-ci`

The `make isort` target will also fix errors, and is intended to be
used locally. The `make isort-ci` target will only alert on errors.

* Sort unsorted imports

.github/workflows/safe_pr_checks.yml

@@ -18,6 +18,9 @@ jobs:
       - name: Format
         run: make black-ci
 
+      - name: Import Sorting
+        run: make isort-ci
+
       - name: Lint
         run: make pylint
 

CHANGELOG.md

@@ -18,7 +18,13 @@ The types of changes are:
 
 
 ## [Unreleased](https://github.com/ethyca/fidesops/compare/1.5.0...main)
+
+### Developer Experience
+
+* Import ordering is now enforced using [isort](https://pycqa.github.io/isort/) in CI [#533](https://github.com/ethyca/fidesops/pull/533)
+
 ### Docs
+
 * Updated documentation for the user management ui [#530](https://github.com/ethyca/fidesops/pull/530)
 
 ## [1.5.0](https://github.com/ethyca/fidesops/compare/1.4.2...1.5.0) - 2022-05-18
@@ -57,10 +63,10 @@ The types of changes are:
 * Publish Fidesops to Pypi [#491](https://github.com/ethyca/fidesops/pull/491)
 
 
-### Changed 
+### Changed
 * Converted HTTP Status Codes to Starlette constant values [#438](https://github.com/ethyca/fidesops/pull/438)
 * SaasConnector.send behavior on ignore_errors now returns raw response [#462](https://github.com/ethyca/fidesops/pull/462)
-* Seed user permissions in `create_superuser.py` script [#468](https://github.com/ethyca/fidesops/pull/468) 
+* Seed user permissions in `create_superuser.py` script [#468](https://github.com/ethyca/fidesops/pull/468)
 * User API Endpoints (update fields and reset user passwords) [#471](https://github.com/ethyca/fidesops/pull/471)
 * Format tests with `black` [#466](https://github.com/ethyca/fidesops/pull/466)
 * Extract privacy request endpoint logic into separate service for DRP [#470](https://github.com/ethyca/fidesops/pull/470)
@@ -89,4 +95,4 @@ The types of changes are:
 
 ### Developer Experience
 * Add script to seed initial Privacy Request [#487](https://github.com/ethyca/fidesops/pull/487)
-* Add first and last name to `createsuperuser` script [#486](https://github.com/ethyca/fidesops/pull/486)
+* Add first and last name to `createsuperuser` script [#486](https://github.com/ethyca/fidesops/pull/486)

Makefile

@@ -77,7 +77,7 @@ docker-push:
 # CI
 ####################
 
-check-all: black-ci pylint mypy check-migrations pytest pytest-integration
+check-all: isort-ci black-ci pylint mypy check-migrations pytest pytest-integration
 
 black-ci: compose-build
 	@echo "Running black checks..."
@@ -93,6 +93,11 @@ check-migrations: compose-build
 	from fidesops.core.config import config; \
 	check_missing_migrations(config.database.SQLALCHEMY_DATABASE_URI);"
 
+isort-ci:
+	@echo "Running isort checks..."
+	@docker-compose run $(IMAGE_NAME) \
+		isort src tests --check-only
+
 pylint: compose-build
 	@echo "Running pylint checks..."
 	@docker-compose run $(IMAGE_NAME) \
@@ -155,6 +160,12 @@ compose-build:
 	@docker-compose down --remove-orphans
 	@docker-compose build --build-arg REQUIRE_MSSQL="true"
 
+.PHONY: isort
+isort:
+	@echo "Running isort checks..."
+	@docker-compose run $(IMAGE_NAME) \
+		isort src tests
+
 .PHONY: teardown
 teardown:
 	@echo "Tearing down the dev environment..."

dev-requirements.txt

@@ -1,12 +1,13 @@
-Faker==8.12.1
 black==21.8b0
+Faker==8.12.1
+ipython
+isort
+mypy==0.812
 packaging==20.9
+pdbpp
 pre-commit==2.9.3
 pylint==2.6.0
-pytest==6.2.2
 pytest-cov==2.11.1
-requests-mock==1.8.0
-mypy==0.812
-pdbpp
-ipython
 pytest-env==0.6.2
+pytest==6.2.2
+requests-mock==1.8.0

pyproject.toml

@@ -33,6 +33,14 @@ exclude = '''
 )/
 '''
 
+#######
+# isort
+#######
+[tool.isort]
+profile = "black"
+line_length = 88
+src_paths = ["src", "tests"]
+
 ########
 # Pylint
 ########

src/fidesops/api/v1/api.py

@@ -12,13 +12,12 @@
     policy_endpoints,
     policy_webhook_endpoints,
     privacy_request_endpoints,
-    storage_endpoints,
     saas_config_endpoints,
+    storage_endpoints,
     user_endpoints,
     user_permission_endpoints,
 )
 
-
 api_router = APIRouter()
 api_router.include_router(config_endpoints.router)
 api_router.include_router(connection_endpoints.router)

src/fidesops/api/v1/endpoints/config_endpoints.py

@@ -1,22 +1,14 @@
 import logging
-from typing import (
-    Any,
-    Dict,
-)
+from typing import Any, Dict
 
 from fastapi import APIRouter
 from fastapi.params import Security
 
-from fidesops.api.v1 import (
-    scope_registry as scopes,
-    urn_registry as urls,
-)
-from fidesops.core.config import (
-    censored_config,
-)
+from fidesops.api.v1 import scope_registry as scopes
+from fidesops.api.v1 import urn_registry as urls
+from fidesops.core.config import censored_config
 from fidesops.util.oauth_util import verify_oauth_client
 
-
 router = APIRouter(tags=["Config"], prefix=urls.V1_URL_PREFIX)
 
 logger = logging.getLogger(__name__)

src/fidesops/api/v1/endpoints/dataset_endpoints.py

@@ -1,7 +1,7 @@
 import logging
 from typing import List
-import yaml
 
+import yaml
 from fastapi import APIRouter, Depends, HTTPException, Request
 from fastapi.params import Security
 from fastapi_pagination import Page, Params
@@ -27,8 +27,8 @@
     DATASET_BY_KEY,
     DATASET_VALIDATE,
     DATASETS,
-    YAML_DATASETS,
     V1_URL_PREFIX,
+    YAML_DATASETS,
 )
 from fidesops.common_exceptions import (
     SaaSConfigNotFoundException,

src/fidesops/api/v1/endpoints/drp_endpoints.py

@@ -1,14 +1,14 @@
 import logging
-from typing import Dict, Any, Optional
+from typing import Any, Dict, Optional
 
 import jwt
-from fastapi import HTTPException, Depends, APIRouter, Security
+from fastapi import APIRouter, Depends, HTTPException, Security
 from sqlalchemy.orm import Session
 from starlette.status import (
+    HTTP_200_OK,
     HTTP_404_NOT_FOUND,
     HTTP_422_UNPROCESSABLE_ENTITY,
     HTTP_424_FAILED_DEPENDENCY,
-    HTTP_200_OK,
     HTTP_500_INTERNAL_SERVER_ERROR,
 )
 
@@ -19,7 +19,7 @@
 from fidesops.core.config import config
 from fidesops.models.policy import Policy
 from fidesops.models.privacy_request import PrivacyRequest
-from fidesops.schemas.drp_privacy_request import DrpPrivacyRequestCreate, DrpIdentity
+from fidesops.schemas.drp_privacy_request import DrpIdentity, DrpPrivacyRequestCreate
 from fidesops.schemas.privacy_request import PrivacyRequestDRPStatusResponse
 from fidesops.schemas.redis_cache import PrivacyRequestIdentity
 from fidesops.service.drp.drp_fidesops_mapper import DrpFidesopsMapper

src/fidesops/api/v1/endpoints/encryption_endpoints.py

@@ -1,29 +1,29 @@
+import logging
 import secrets
 
-import logging
 from fastapi import APIRouter, Security
 
 from fidesops.api.v1.scope_registry import ENCRYPTION_EXEC
 from fidesops.api.v1.urn_registry import (
+    DECRYPT_AES,
     ENCRYPT_AES,
     ENCRYPTION_KEY,
-    DECRYPT_AES,
     V1_URL_PREFIX,
 )
 from fidesops.core.config import config
 from fidesops.schemas.encryption_request import (
+    AesDecryptionRequest,
+    AesDecryptionResponse,
     AesEncryptionRequest,
     AesEncryptionResponse,
-    AesDecryptionResponse,
-    AesDecryptionRequest,
 )
 from fidesops.util import cryptographic_util
-from fidesops.util.cryptographic_util import bytes_to_b64_str, b64_str_to_bytes
+from fidesops.util.cryptographic_util import b64_str_to_bytes, bytes_to_b64_str
 from fidesops.util.encryption.aes_gcm_encryption_scheme import (
-    encrypt_verify_secret_length as aes_gcm_encrypt,
+    decrypt as aes_gcm_decrypt,
 )
 from fidesops.util.encryption.aes_gcm_encryption_scheme import (
-    decrypt as aes_gcm_decrypt,
+    encrypt_verify_secret_length as aes_gcm_encrypt,
 )
 from fidesops.util.oauth_util import verify_oauth_client
 

src/fidesops/api/v1/endpoints/masking_endpoints.py

@@ -2,19 +2,18 @@
 from typing import List
 
 from fastapi import APIRouter, HTTPException
-from starlette.status import HTTP_404_NOT_FOUND, HTTP_400_BAD_REQUEST
+from starlette.status import HTTP_400_BAD_REQUEST, HTTP_404_NOT_FOUND
 
 from fidesops.api.v1.urn_registry import MASKING, MASKING_STRATEGY, V1_URL_PREFIX
 from fidesops.common_exceptions import ValidationError
-from fidesops.schemas.masking.masking_api import MaskingAPIResponse, MaskingAPIRequest
+from fidesops.schemas.masking.masking_api import MaskingAPIRequest, MaskingAPIResponse
 from fidesops.schemas.masking.masking_strategy_description import (
     MaskingStrategyDescription,
 )
-
 from fidesops.service.masking.strategy.masking_strategy_factory import (
+    NoSuchStrategyException,
     get_strategies,
     get_strategy,
-    NoSuchStrategyException,
 )
 
 router = APIRouter(tags=["Masking"], prefix=V1_URL_PREFIX)

src/fidesops/common_exceptions.py

@@ -1,10 +1,9 @@
 from typing import List
 
 from fastapi import HTTPException
-
 from starlette.status import (
-    HTTP_401_UNAUTHORIZED,
     HTTP_400_BAD_REQUEST,
+    HTTP_401_UNAUTHORIZED,
     HTTP_403_FORBIDDEN,
     HTTP_404_NOT_FOUND,
 )

src/fidesops/core/config.py

@@ -3,17 +3,11 @@
 import hashlib
 import logging
 import os
-from typing import Dict, List, Optional, Union, Tuple, Any, MutableMapping
+from typing import Any, Dict, List, MutableMapping, Optional, Tuple, Union
 
 import bcrypt
 import toml
-from pydantic import (
-    AnyHttpUrl,
-    BaseSettings,
-    PostgresDsn,
-    ValidationError,
-    validator,
-)
+from pydantic import AnyHttpUrl, BaseSettings, PostgresDsn, ValidationError, validator
 from pydantic.env_settings import SettingsSourceCallable
 
 from fidesops.common_exceptions import MissingConfig

src/fidesops/db/base.py

@@ -2,12 +2,12 @@
 # Import all the models, so that Base has them before being
 # imported by Alembic
 from fidesops.db.base_class import Base
+from fidesops.models.audit_log import AuditLog
 from fidesops.models.client import ClientDetail
 from fidesops.models.connectionconfig import ConnectionConfig
 from fidesops.models.datasetconfig import DatasetConfig
+from fidesops.models.fidesops_user import FidesopsUser
+from fidesops.models.fidesops_user_permissions import FidesopsUserPermissions
 from fidesops.models.policy import Policy, Rule, RuleTarget
 from fidesops.models.privacy_request import PrivacyRequest
 from fidesops.models.storage import StorageConfig
-from fidesops.models.fidesops_user import FidesopsUser
-from fidesops.models.audit_log import AuditLog
-from fidesops.models.fidesops_user_permissions import FidesopsUserPermissions

src/fidesops/db/base_class.py

@@ -7,14 +7,10 @@
 from uuid import uuid4
 
 import six
-
-from sqlalchemy import Column, String, DateTime
+from sqlalchemy import Column, DateTime, String
 from sqlalchemy.ext.declarative import declarative_base, declared_attr
 from sqlalchemy.ext.mutable import MutableDict
-from sqlalchemy.orm import (
-    Session,
-    Query,
-)
+from sqlalchemy.orm import Query, Session
 from sqlalchemy.sql import func
 from sqlalchemy_utils import JSONType
 

src/fidesops/db/database.py

@@ -2,13 +2,14 @@
 Contains all of the logic for spinning up/tearing down the database.
 """
 import os
-from pydantic import PostgresDsn
 
 from alembic import command
 from alembic.config import Config
 from alembic.migration import MigrationContext
+from pydantic import PostgresDsn
 
 from fidesops.db.session import get_db_engine
+
 from .base import Base
 
 

src/fidesops/db/session.py

@@ -3,10 +3,8 @@
 
 from sqlalchemy import create_engine
 from sqlalchemy.engine import Engine
-from sqlalchemy.orm import (
-    sessionmaker,
-    Session,
-)
+from sqlalchemy.orm import Session, sessionmaker
+
 from fidesops.core.config import config
 
 logger = logging.getLogger(__name__)

src/fidesops/graph/config.py

@@ -80,14 +80,15 @@
 from abc import ABC, abstractmethod
 from collections import defaultdict
 from dataclasses import dataclass
-from typing import List, Literal, Optional, Tuple, Set, Dict, Any, Callable
+from typing import Any, Callable, Dict, List, Literal, Optional, Set, Tuple
+
 from pydantic import BaseModel, validator
 
 from fidesops.common_exceptions import FidesopsException
 from fidesops.graph.data_type import (
+    DataType,
     DataTypeConverter,
     get_data_type_converter,
-    DataType,
 )
 from fidesops.schemas.shared_schemas import FidesOpsKey
 from fidesops.util.collection_util import merge_dicts

src/fidesops/graph/data_type.py

@@ -1,7 +1,7 @@
 import logging
-from abc import abstractmethod, ABC
+from abc import ABC, abstractmethod
 from enum import Enum
-from typing import Generic, Optional, Any, TypeVar, Dict, Tuple, Set
+from typing import Any, Dict, Generic, Optional, Set, Tuple, TypeVar
 
 from bson.errors import InvalidId
 from bson.objectid import ObjectId