Use bandit, pylint, safety, mypy, and pyproject.toml - coll…

…ection factory (#119)

Add/remove pre-commit hooks:
Remove: `flake8`
Add: `bandit`, `pylint`, `mypy`, `pytest-plugins` (local)
Update `black` hook to the latest version.

Run and satisfy the new tools.

Use `typing.TYPE_CHECKING` throughout.
Since FastAPI depends on pydantic and relies on the type definitions to
function, the routers cannot use `TYPE_CHECKING`. However, it is used
to the extent it can be done.

Create a factory for entry-endpoint resource collections.
In order to avoid cyclic imports and clean up the code, an
entry-endpoint resource collection factory has been implemented.
It updates a global dictionary, where the key is the MongoDB collection
name configured and retrieved from `CONFIG`. The value is then an
asynchronous MongoDB collection similar to the `MongoCollection` class
in OPTIMADE Python tools.
With this, the `*_COLLECTIONS` can be removed throughout as well.

Add new CI job for `pylint` and `safety`.
`pylint` relies on having access to imported packages.
It is therefore skipped in the `pre-commit` job and given its own.

Make sure to import types for building docs.
This is done by setting the environment variable `MKDOCS_BUILD`
when building the docs. It is discarded after the build.

Don't use doc-strings through from `optimade`:
There was an issue with this, where a local API reference was made,
which couldn't be resolved when building these docs.

Use `pyproject.toml` throughout (remove `setup.cfg`).

Add `invoke` task to update pytest config plugins.
The task updates the `required_plugins` config option for `pytest` with
the versions specified in `requirements_dev.txt`.
It is implemented as a `pre-commit` hook as well with the id
`pytest-plugins`.

.github/workflows/ci.yml

@@ -29,13 +29,43 @@ jobs:
         while IFS="" read -r line || [ -n "${line}" ]; do
             if [[ "${line}" =~ ^pre-commit.*$ ]]; then
                 pre_commit="${line}"
+            elif [[ "${line}" =~ ^pylint.*$ ]]; then
+                pylint="${line}"
+            elif [[ "${line}" =~ ^invoke.*$ ]]; then
+                invoke="${line}"
             fi
         done < requirements_dev.txt
 
-        pip install ${pre_commit}
+        pip install ${pre_commit} ${pylint} ${invoke}
 
     - name: Test with pre-commit
-      run: pre-commit run --all-files || ( git status --short ; git diff ; exit 1 )
+      run: SKIP=pylint pre-commit run --all-files --show-diff-on-failure
+
+  pylint-safety:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        fetch-depth: 2
+
+    - name: Set up Python 3.8
+      uses: actions/setup-python@v2
+      with:
+        python-version: 3.8
+
+    - name: Install dependencies
+      run: |
+        python -m pip install -U pip
+        pip install -U setuptools
+        pip install -U -r requirements.txt -r requirements_dev.txt -r requirements_docs.txt
+        pip install safety
+
+    - name: Run pylint
+      run: pylint --rcfile=pyproject.toml *.py optimade_gateway
+
+    - name: Run safety
+      run: pip freeze | safety check --stdin
 
   pytest:
     runs-on: ubuntu-latest

.gitignore

@@ -1,4 +1,14 @@
-logs/
+# Tools
 .coverage
 htmlcov/
+.mypy*
+.pytest*
+
+# Python
+*.egg*
+
+# Application
+logs/
+
+# Documentation
 site/

.pre-commit-config.yaml

@@ -7,13 +7,40 @@ repos:
     exclude: .*\.md$
 
 - repo: https://github.com/ambv/black
-  rev: 21.7b0
+  rev: 21.8b0
   hooks:
   - id: black
     name: Blacken
 
-- repo: https://gitlab.com/pycqa/flake8
-  rev: '3.9.2'
+- repo: https://github.com/PyCQA/bandit
+  rev: '1.7.0'
   hooks:
-  - id: flake8
-    args: [--count, --show-source, --statistics]
+  - id: bandit
+    args: [-r]
+    exclude: ^tests/.*$
+
+- repo: https://github.com/pre-commit/mirrors-mypy
+  rev: v0.910
+  hooks:
+  - id: mypy
+    exclude: ^tests/.*$
+
+- repo: local
+  hooks:
+  - id: pylint
+    name: pylint
+    entry: pylint
+    language: python
+    types: [python]
+    require_serial: true
+    exclude: ^tests/.*$
+  - id: pytest-plugins
+    name: pytest required plugin versions
+    entry: invoke
+    args: [update-pytest-reqs]
+    language: python
+    always_run: true
+    types: [text]
+    pass_filenames: false
+    files: ^requirements_dev.txt$
+    description: Update the pytest plugins to be minimum the currently listed requirement versions.

mkdocs.yml

@@ -68,12 +68,17 @@ plugins:
               - "!__json_encoder__$"
               - "!__all__$"
               - "!__config__$"
+              - "!__str__$"
+              - "!__repr__$"
               - "!ValidatorResults$"
             members: true
             inherited_members: false
             docstring_style: google
             docstring_options:
               replace_admonitions: true
+          setup_commands:
+            - import os
+            - os.environ["MKDOCS_BUILD"] = "1"
       watch:
         - optimade_gateway
   - awesome-pages

optimade_gateway/common/config.py

@@ -1,8 +1,8 @@
-"""Configuration of the FastAPI server"""
-# pylint: disable=no-self-argument
+"""Configuration of the FastAPI server."""
+# pylint: disable=no-self-use,no-self-argument
 import os
 import re
-import warnings
+from warnings import warn
 
 from optimade.server.config import ServerConfig as OptimadeServerConfig
 from pydantic import Field, validator
@@ -32,26 +32,30 @@ class ServerConfig(OptimadeServerConfig):
         True,
         description=(
             "Whether or not to load all valid OPTIMADE providers' databases from the "
-            "[Materials-Consortia list of OPTIMADE providers](https://providers.optimade.org) on "
-            "server startup."
+            "[Materials-Consortia list of OPTIMADE providers]"
+            "(https://providers.optimade.org) on server startup."
         ),
     )
 
     @validator("mongo_uri")
-    def replace_with_env_vars(cls, v):
+    def replace_with_env_vars(cls, value: str) -> str:
         """Replace string variables with environment variables, if possible"""
-        res: str = v
-        for match in re.finditer(r"\{[^{}]+\}", v):
+        res = value
+        for match in re.finditer(r"\{[^{}]+\}", value):
             string_var = match.group()[1:-1]
             env_var = os.getenv(
                 string_var, os.getenv(string_var.upper(), os.getenv(string_var.lower()))
             )
             if env_var is not None:
                 res = res.replace(match.group(), env_var)
             else:
-                warnings.warn(
-                    f"Could not find an environment variable for {match.group()!r} from mongo_uri: {v}",
-                    OptimadeGatewayWarning,
+                warn(
+                    OptimadeGatewayWarning(
+                        detail=(
+                            "Could not find an environment variable for "
+                            f"{match.group()!r} from mongo_uri: {value}"
+                        )
+                    )
                 )
         return res
 

optimade_gateway/common/exceptions.py

@@ -1,3 +1,4 @@
+"""Specific OPTIMADE Gateway Python exceptions."""
 __all__ = ("OptimadeGatewayError",)
 
 

optimade_gateway/common/logger.py

@@ -4,9 +4,14 @@
 import os
 from pathlib import Path
 import sys
+from typing import TYPE_CHECKING
 
 from uvicorn.logging import DefaultFormatter
 
+if TYPE_CHECKING or bool(os.getenv("MKDOCS_BUILD", "")):  # pragma: no cover
+    # pylint: disable=ungrouped-imports
+    import logging.handlers
+
 
 @contextmanager
 def disable_logging():

optimade_gateway/common/utils.py

@@ -1,16 +1,27 @@
+"""Common utility functions.
+
+These functions may be used in general throughout the OPTIMADE Gateway Python code.
+"""
+# pylint: disable=line-too-long
 from enum import Enum
-from typing import Any, Dict, Union
+from os import getenv
+from typing import TYPE_CHECKING
+
+from pydantic import BaseModel
 
-from pydantic import BaseModel  # pylint: disable=no-name-in-module
+if TYPE_CHECKING or bool(getenv("MKDOCS_BUILD", "")):  # pragma: no cover
+    # pylint: disable=unused-import
+    from typing import Any, Dict, Union
 
 
-async def clean_python_types(data: Any) -> Any:
+async def clean_python_types(data: "Any") -> "Any":
     """Turn any types into MongoDB-friendly Python types.
 
     Use `dict()` method for Pydantic models.
     Use `value` property for Enums.
     Turn tuples and sets into lists.
     """
+    res: "Any" = None
     if isinstance(data, (list, tuple, set)):
         res = []
         for datum in data:
@@ -33,11 +44,11 @@ async def clean_python_types(data: Any) -> Any:
 
 
 def get_resource_attribute(
-    resource: Union[BaseModel, Dict[str, Any], None],
+    resource: "Union[BaseModel, Dict[str, Any], None]",
     field: str,
-    default: Any = None,
+    default: "Any" = None,
     disambiguate: bool = True,
-) -> Any:
+) -> "Any":
     """Return a resource's field's value
 
     Get the field value no matter if the resource is a pydantic model or a Python dictionary.
@@ -57,8 +68,8 @@ def get_resource_attribute(
             `"attributes.base_url"`.
         default: The default value to return if `field` does not exist.
         disambiguate: Whether or not to "shortcut" a field value.
-            For example, for `attributes.base_url`, if `True`, this would return the string value
-            or the value of it's `"href"` key.
+            For example, for `attributes.base_url`, if `True`, this would return the
+            string value or the value of it's `"href"` key.
 
     Returns:
         The resource's field's value.
@@ -68,21 +79,21 @@ def get_resource_attribute(
         _get_attr = getattr
     elif isinstance(resource, dict):
 
-        def _get_attr(mapping: dict, key: str, default: Any) -> Any:
+        def _get_attr(mapping: dict, key: str, default: "Any") -> "Any":  # type: ignore[misc]
             return mapping.get(key, default)
 
     elif resource is None:
         # Allow passing `None`, but simply return `default`
         return default
     else:
         raise TypeError(
-            "resource must be either a pydantic model or a Python dictionary, it was of type "
-            f"{type(resource)!r}"
+            "resource must be either a pydantic model or a Python dictionary, it was of "
+            f"type {type(resource)!r}"
         )
 
     fields = field.split(".")
-    for field in fields[:-1]:
-        resource = _get_attr(resource, field, {})
+    for _ in fields[:-1]:
+        resource = _get_attr(resource, _, {})
     field = fields[-1]
     value = _get_attr(resource, field, default)