refactor dev & setup workflows

* refactor package setup - drop setup.py & setup.cfg - use pyproject.toml - remove deprecated config code * use bump-my-version for releases bump2version being deprecated, bump-my-version is a maintained fork. * improve git & lint workflows - add pre-commit - use black, ruff & mypy via pre-commit - fix some lint issues catched by ruff * refactor dev workflows - drop scripts folder - add Makefile - add dev optional dependency - remove all requirements files - generate new requirements-dev.txt * add pre-commit hooks - debug-statements - vulture (and drop RUF100 * restrict scopes of github workflows * refactor dev workflows - only hard-pin test dependencies - add/update/drop Makefile recipes - adapt github workflows
Cornerstone-OnDemand · Sep 26, 2023 · 1db6781 · 1db6781
1 parent 38e5c15
commit 1db6781
Show file tree

Hide file tree

Showing 40 changed files with 520 additions and 633 deletions.
diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
@@ -2,7 +2,7 @@ name: docs
 
 on:
   workflow_run:
-    workflows: ["CI"]
+    workflows: ["tests"]
     types:
       - completed
 
@@ -14,12 +14,12 @@ jobs:
     steps:
       - name: Checkout Repository
         uses: actions/checkout@v3
-    
+
       - name: Setup Python
         uses: actions/setup-python@v3
         with:
           python-version: 3.x
-          
+
       - name: Install dependencies
         run: pip install mkdocs-material mkdocs-git-revision-date-localized-plugin
 
@@ -36,7 +36,7 @@ jobs:
       - name: Download Coverage Report
         uses: dawidd6/action-download-artifact@v2
         with:
-          workflow: main.yml
+          workflow: tests.yml
           name: coverage
           path: coverage
           search_artifacts: true

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -1,6 +1,10 @@
 name: lint
 
-on: [push, pull_request]
+on:
+  push:
+    branches: ["main"]
+  pull_request:
+    branches: ["main"]
 
 jobs:
   lint:
@@ -9,13 +13,11 @@ jobs:
     steps:
       - name: Checkout repository
         uses: actions/checkout@v3
-      - name: Set up Python 3.11
+      - name: Set up Python 3.8
         uses: actions/setup-python@v3
         with:
-          python-version: "3.11"
-      - name: Install dependencies
-        run: python3.11 -m pip install -r requirements-dev.txt
-      - name: Install mypy types
-        run: mypy --install-types --non-interactive -p modelkit
+          python-version: "3.8"
+      - name: Setup lint
+        run: make setup_lint
       - name: Run lint
-        run: scripts/lint.sh
+        run: make lint
diff --git a/.github/workflows/main.yml → .github/workflows/tests.yml b/.github/workflows/main.yml → .github/workflows/tests.yml
@@ -1,13 +1,17 @@
-name: CI
-
-on: [push, pull_request]
+name: tests
 
 defaults:
   run:
     shell: bash
 
+on:
+  push:
+    branches: ["main"]
+  pull_request:
+    branches: ["main"]
+
 jobs:
-  test:
+  tests:
     strategy:
       fail-fast: false
       matrix:
@@ -25,11 +29,8 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
 
-      - name: Install Dependencies
-        run: python -m pip install --upgrade nox
-
       - name: Run Tests
-        run: scripts/ci_tests.sh
+        run: make ci_tests
         env:
           OS_NAME: "${{ matrix.os }}"
           PYTHON_VERSION: "${{ matrix.python-version }}"

diff --git a/.gitignore b/.gitignore
@@ -76,6 +76,9 @@ output/*/index.html
 # pytest
 .pytest_cache
 
+# ruff
+.ruff_cache
+
 # tmp files
 /tmp/
 /*.json

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,47 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.0.1
+    hooks:
+      # - id: trailing-whitespace # TODO: enable this
+      # - id: end-of-file-fixer  # TODO: enable this
+      - id: debug-statements
+
+  - repo: https://github.com/psf/black
+    rev: 23.7.0
+    hooks:
+      - id: black
+        args: ["--target-version", "py38"]
+
+  - repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: 'v0.0.290'
+    hooks:
+      - id: ruff
+        args: ["--fix"]
+
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.5.1
+    hooks:
+      - id: mypy
+        types: [python]
+        args: []  # TODO: enable this: "--check-untyped-defs"
+        # TODO: narrow tests exclusion down to: ^tests/testdata/typing/*$
+        exclude: |
+          (?x)^(
+              .vulture.py|
+              tests/.*
+          )$
+        additional_dependencies: [
+          pydantic==1.*,
+          types-python-dateutil,
+          types-requests,
+          types-urllib3,
+          types-redis,
+          types-cachetools,
+          types-tabulate
+        ]
+
+  - repo: https://github.com/jendrikseipp/vulture
+    rev: v2.9.1
+    hooks:
+      - id: vulture
+        args: ["--min-confidence", "90", "modelkit", "bin", ".vulture.py"]
diff --git a/.vulture.py b/.vulture.py
diff --git a/MANIFEST.in b/MANIFEST.in
@@ -0,0 +1,3 @@
+recursive-exclude * __pycache__
+recursive-exclude * *.py[co]
+recursive-exclude tests *
diff --git a/Makefile b/Makefile
@@ -0,0 +1,60 @@
+SHELL := /bin/bash
+.SHELLFLAGS := -e -x -c -o pipefail
+.PHONY: setup setup_lint lint tests coverage ci_tests requirements upgrade
+
+setup_lint:
+	pip install --upgrade pip pre-commit
+	pre-commit install
+
+lint:
+	pre-commit run --all-files
+
+tests:
+	export ENABLE_TF_SERVING_TEST=True; \
+	export ENABLE_TF_TEST=True; \
+	export ENABLE_REDIS_TEST=True; \
+	export ENABLE_S3_TEST=True; \
+	export ENABLE_GCS_TEST=True; \
+	export ENABLE_AZ_TEST=True; \
+	pytest
+
+coverage:
+	export ENABLE_TF_SERVING_TEST=True; \
+	export ENABLE_TF_TEST=True; \
+	export ENABLE_REDIS_TEST=True; \
+	export ENABLE_S3_TEST=True; \
+	export ENABLE_GCS_TEST=True; \
+	export ENABLE_AZ_TEST=True; \
+	coverage run -m pytest; \
+	coverage report -m; \
+	coverage xml
+
+setup:
+	pip install --upgrade pip
+	pip install -r requirements-dev.txt
+	pip install -e .[lint,tensorflow,cli,api,assets-s3,assets-gcs,assets-az]
+	pre-commit install
+
+ci_tests:
+	pip install --upgrade pip nox
+	@if [ "$(OS_NAME)" = "ubuntu-latest" ]; then \
+		export ENABLE_TF_SERVING_TEST=True; \
+		export ENABLE_TF_TEST=True; \
+		export ENABLE_REDIS_TEST=True; \
+		export ENABLE_S3_TEST=True; \
+		export ENABLE_GCS_TEST=True; \
+		export ENABLE_AZ_TEST=True; \
+		nox --error-on-missing-interpreters -s "coverage-$(PYTHON_VERSION)"; \
+	else \
+		nox --error-on-missing-interpreters -s "tests-$(PYTHON_VERSION)"; \
+	fi
+
+requirements:
+	pip install --upgrade pip pip-compile
+	pip-compile --extra=dev --output-file=requirements-dev.txt
+
+upgrade:
+	pip install --upgrade pip pip-tools pre-commit
+	pre-commit autoupdate
+	pip-compile --upgrade --extra=dev --output-file=requirements-dev.txt
+	pip install -r requirements-dev.txt
diff --git a/docs/contributions/release.md b/docs/contributions/release.md
@@ -10,9 +10,13 @@ git switch -C releasing central/main
 
 ## Bump version
 
-We use `bump2version` to create the commit and tag required for the release
+We use `bump-my-version` to create the commit and tag required for the release
 ```bash
-bump2version patch
+bump-my-version bump patch
+```
+or via its alias:
+```bash
+bumpversion bump patch
 ```
 
 ## Push commit and tag to central
@@ -21,7 +25,7 @@ bump2version patch
 git push --follow-tags central releasing:main
 
 # Or more concisely if you have configured git with push.default = upstream
-git push --follow-tags 
+git push --follow-tags
 ```
 
 ## Package and publish new artifact to pypi

diff --git a/modelkit/api.py b/modelkit/api.py
@@ -129,15 +129,15 @@ def _make_model_endpoint_fn(self, model, item_type):
             async def _aendpoint(
                 item: item_type = fastapi.Body(...),
                 model=fastapi.Depends(lambda: self.lib.get(model.configuration_key)),
-            ):  # noqa: B008
+            ):
                 return await model.predict(item)
 
             return _aendpoint
 
         def _endpoint(
             item: item_type = fastapi.Body(...),
             model=fastapi.Depends(lambda: self.lib.get(model.configuration_key)),
-        ):  # noqa: B008
+        ):
             return model.predict(item)
 
         return _endpoint
@@ -148,15 +148,15 @@ def _make_batch_model_endpoint_fn(self, model, item_type):
             async def _aendpoint(
                 item: List[item_type] = fastapi.Body(...),
                 model=fastapi.Depends(lambda: self.lib.get(model.configuration_key)),
-            ):  # noqa: B008
+            ):
                 return await model.predict_batch(item)
 
             return _aendpoint
 
         def _endpoint(
             item: List[item_type] = fastapi.Body(...),
             model=fastapi.Depends(lambda: self.lib.get(model.configuration_key)),
-        ):  # noqa: B008
+        ):
             return model.predict_batch(item)
 
         return _endpoint

diff --git a/modelkit/assets/drivers/gcs.py b/modelkit/assets/drivers/gcs.py
@@ -72,14 +72,14 @@ def download_object(self, object_name, destination_path):
         try:
             with open(destination_path, "wb") as f:
                 blob.download_to_file(f)
-        except NotFound:
+        except NotFound as e:
             logger.error(
                 "Object not found.", bucket=self.bucket, object_name=object_name
             )
             os.remove(destination_path)
             raise errors.ObjectDoesNotExistError(
                 driver=self, bucket=self.bucket, object_name=object_name
-            )
+            ) from e
 
     @retry(**GCS_RETRY_POLICY)
     def delete_object(self, object_name):

diff --git a/modelkit/assets/drivers/s3.py b/modelkit/assets/drivers/s3.py
@@ -83,14 +83,14 @@ def download_object(self, object_name, destination_path):
         try:
             with open(destination_path, "wb") as f:
                 self.client.download_fileobj(self.bucket, object_name, f)
-        except botocore.exceptions.ClientError:
+        except botocore.exceptions.ClientError as e:
             logger.error(
                 "Object not found.", bucket=self.bucket, object_name=object_name
             )
             os.remove(destination_path)
             raise errors.ObjectDoesNotExistError(
                 driver=self, bucket=self.bucket, object_name=object_name
-            )
+            ) from e
 
     @retry(**S3_RETRY_POLICY)
     def delete_object(self, object_name):

diff --git a/modelkit/core/errors.py b/modelkit/core/errors.py
@@ -114,8 +114,8 @@ def wrapper(*args, **kwargs):
                 return func(*args, **kwargs)
             except PredictionError as exc:
                 if os.environ.get("MODELKIT_ENABLE_SIMPLE_TRACEBACK", "True") == "True":
-                    raise strip_modelkit_traceback_frames(exc.exc)
-                raise exc.exc
+                    raise strip_modelkit_traceback_frames(exc.exc) from exc
+                raise exc.exc from exc
             except BaseException:
                 raise
 
@@ -132,8 +132,8 @@ def wrapper(*args, **kwargs):
                 yield from func(*args, **kwargs)
             except PredictionError as exc:
                 if os.environ.get("MODELKIT_ENABLE_SIMPLE_TRACEBACK", "True") == "True":
-                    raise strip_modelkit_traceback_frames(exc.exc)
-                raise exc.exc
+                    raise strip_modelkit_traceback_frames(exc.exc) from exc
+                raise exc.exc from exc
             except BaseException:
                 raise
 
@@ -150,8 +150,8 @@ async def wrapper(*args, **kwargs):
                 return await func(*args, **kwargs)
             except PredictionError as exc:
                 if os.environ.get("MODELKIT_ENABLE_SIMPLE_TRACEBACK", "True") == "True":
-                    raise strip_modelkit_traceback_frames(exc.exc)
-                raise exc.exc
+                    raise strip_modelkit_traceback_frames(exc.exc) from exc
+                raise exc.exc from exc
             except BaseException:
                 raise
 
@@ -170,8 +170,8 @@ async def wrapper(*args, **kwargs):
                     yield x
             except PredictionError as exc:
                 if os.environ.get("MODELKIT_ENABLE_SIMPLE_TRACEBACK", "True") == "True":
-                    raise strip_modelkit_traceback_frames(exc.exc)
-                raise exc.exc
+                    raise strip_modelkit_traceback_frames(exc.exc) from exc
+                raise exc.exc from exc
             except BaseException:
                 raise
 

diff --git a/modelkit/core/library.py b/modelkit/core/library.py
@@ -107,7 +107,7 @@ def __init__(
                     self.cache = RedisCache(
                         self.settings.cache.host, self.settings.cache.port
                     )
-                except (ConnectionError, redis.ConnectionError):
+                except (ConnectionError, redis.ConnectionError) as e:
                     logger.error(
                         "Cannot ping redis instance",
                         cache_host=self.settings.cache.host,
@@ -117,7 +117,7 @@ def __init__(
                         "Cannot ping redis instance"
                         f"[cache_host={self.settings.cache.host}, "
                         f"port={self.settings.cache.port}]"
-                    )
+                    ) from e
             if isinstance(self.settings.cache, NativeCacheSettings):
                 self.cache = NativeCache(
                     self.settings.cache.implementation, self.settings.cache.maxsize
@@ -354,7 +354,7 @@ def _resolve_assets(self, model_name):
 
     def preload(self):
         # make sure the assets_manager is instantiated
-        self.assets_manager
+        _ = self.assets_manager
         for model_name in self.required_models:
             self._load(model_name)