Add Mypy Static Type Checking to SDK & Pull latest bentobox-engine im…

…age as build cache (#61)

* feat(build): Add mypy to project makefile to lint types when linting SDK

* build(sdk): Update requirements-dev.txt to include mypy

* refactor(sdk): Fix or ignore errors flagged by mypy.

* repo: Add secrets, end of file, trailing whitespace, merge confict and secrets pre commit checks

* refactor(ci/cd): Replace docker image caching in favor of just pulling the previous docker image

* feat(cd): Also push bentobox-engine to the latest tag on publish-engine job

* refactor(build): Execute mypy standalone instead of via python -m

* fix(ci): Fix typos causing pull image for cache step to fail, temporary run CD in PR for testing

* fix(sdk): Fix failing mypy due by removing unneeded typing-protobuf package

* chore(sdk): Remove uneeded type: ignores and clean up types

* fix(build): Fix project makefile's lint-sdk failing  due to mypy missing type annoatations for protobuf bindings

* fix(build): Fix missing cache metadata when required to use pulled images as cache

* fix(ci): Fix missing protoc required to run make lint-sdk

* refactor(ci): Run lint-sdk as a seperate job from build-test-sdk

Simplifies as we do not need to support linting on all OSes. linting
only looks for the problems in the codebase which is OS-independent, so
linting on one OS should be sufficient

* fix(ci): Fix build-test-sdk still install protoc and protoc-gen-mypy not in PATH

* fix(cd): Fix missing docker tag bentobox-engine image as latest

* chore(ci): Add name to to lint-sdk CI job

* fix(ci): Move GITHUB_PATH setting to previous action in job as PATH is only updated for subsequent jobs

* refactor(sdk): Bump numpy to 1.20.1 to get builtin type annotations

* fix(ci/cd): Use docker buildkit as docker cache from pulled images only works for buildkit images

* Revert "refactor(sdk): Bump numpy to 1.20.1 to get builtin type annotations"

This reverts commit 95462e1.

* fix(ci): Use setup-python to setup python in lint-sdk instead of using runner python

* feat(ci/cd): Add cache-from flag to use cached latest image as build cache

* fix(build): Fix mypy unable to find generated stubs for Python protobuf bindings

* fix(build): Fix mypy unable to find generated stubs for Python protobuf bindings by installing sdk before lint

* fix(build): Fix missing dep dependencies when using make install-sdk

* chore(cd): Remove temporary on: section used for testing CD pipeline

.github/workflows/cd.yaml

@@ -4,10 +4,13 @@
 #
 
 name: "CD Pipeline"
-on: 
+on:
   push:
     branches:
       - master
+env:
+  DOCKER_REPO: ghcr.io/bentobox-dev/bentobox-engine
+  DOCKER_BUILDKIT: 1
 jobs:
   # builds & publishes docs for the sdk component to Github
   publish-docs-sdk:
@@ -71,7 +74,7 @@ jobs:
     - id: resolve-tag
       name: "Resolve bentobox-engine container tag"
       run: |
-        DOCKER_TAG=ghcr.io/bentobox-dev/bentobox-engine:$(git describe --long --always)
+        DOCKER_TAG=$DOCKER_REPO:$(git describe --long --always)
         echo $DOCKER_TAG
         echo "::set-output name=container-tag::${DOCKER_TAG}"
     - name: "Build bentobox-engine"
@@ -81,6 +84,7 @@ jobs:
         make \
           SIM_BUILD_TYPE=Release \
           SIM_DOCKER_STAGE=release \
+          SIM_DOCKER_CACHE_FROM="${DOCKER_REPO}:latest" \
           SIM_DOCKER=${DOCKER_TAG} \
           build-sim-docker
     - name: "Authenticate with Github Container Registry"
@@ -91,8 +95,11 @@ jobs:
         echo $GHCR_TOKEN | docker login ghcr.io --username $GHCR_USER --password-stdin
     - name: "Push bentobox-engine container to Github Container Registry"
       run: |
+        # push with both latest and versioned tag
         DOCKER_TAG=${{ steps.resolve-tag.outputs.container-tag }}
         docker push $DOCKER_TAG
+        docker tag $DOCKER_TAG "${DOCKER_REPO}:latest"
+        docker push "${DOCKER_REPO}:latest"
 
   # deploy the engine component on GKE using the currently built image
   deploy-engine:
@@ -102,7 +109,7 @@ jobs:
     env:
       PROJECT_ID: ${{ secrets.GKE_PROJECT }}
       GKE_CLUSTER: k8s-bentobox-demo
-      GKE_ZONE: asia-southeast1-c 
+      GKE_ZONE: asia-southeast1-c
     steps:
     - uses: actions/checkout@v2
     # init gcloud CLI
@@ -132,7 +139,7 @@ jobs:
         git diff
         # deploy using kubectl
         ./kustomize build . | kubectl apply -f -
-      
+
   # build and publish the engine component on github container registry
   publish-sdk:
     runs-on: ubuntu-20.04
@@ -152,7 +159,7 @@ jobs:
           # display built artifacts
           ls sdk/dist/
       - name: Publish bentobox-sdk to Test PyPI
-        uses: pypa/[email protected] 
+        uses: pypa/[email protected]
         with:
           password: ${{ secrets.PYPI_TOKEN }}
           packages_dir: sdk/dist/

.github/workflows/ci.yaml

@@ -5,6 +5,9 @@
 
 name: "CI Pipeline"
 on: push
+env:
+  DOCKER_REPO: ghcr.io/bentobox-dev/bentobox-engine
+  DOCKER_BUILDKIT: 1
 jobs:
   # quick check that protos can be compiled by protoc
   # and lint proto formatting
@@ -58,11 +61,35 @@ jobs:
       - name: "Build bentobox-sim"
         run: |
           # stop at 'build' build stage as it contains the dev tools required to run tests
-          make SIM_BUILD_TYPE=Release SIM_DOCKER_STAGE=build build-sim-docker
+          make SIM_BUILD_TYPE=Debug \
+            SIM_DOCKER_STAGE=build \
+            SIM_DOCKER_CACHE_FROM="${DOCKER_REPO}:latest" \
+            build-sim-docker
       - name: "Unit Test bentobox-sim"
         run: |
           docker run bentobox-sim make test-sim
 
+  # lint the SDK component
+  lint-sdk:
+    name: "Lint bentobox-sdk"
+    needs: check-protos
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: 3.7
+      - name: "Install protoc"
+        run: |
+          mkdir bin
+          make BIN_DIR=bin dep-protoc
+      - name: "Install bentobox-sdk"
+        run: |
+          make install-sdk
+      - name: "Lint bentobox-sdk"
+        run: |
+          make lint-sdk PROTOC=./bin/protoc
+
   # build & unit tests sdk component
   build-test-sdk:
     needs: check-protos
@@ -84,9 +111,6 @@ jobs:
       - name: "Pull dependencies"
         run: |
           make dep-sdk-dev
-      - name: "Lint bentobox-sdk"
-        run: |
-          make lint-sdk
       - name: "Build bentobox-sdk"
         run: |
           make build-sdk
@@ -128,9 +152,12 @@ jobs:
       - name: "Pull dependencies"
         run: |
           make dep-e2e
-      - name: "Build bentobox-sim"
+      - name: "Build bentobox-engine"
         run: |
-          make SIM_BUILD_TYPE=Release SIM_DOCKER_STAGE=release build-sim-docker
+          make SIM_BUILD_TYPE=Release \
+            SIM_DOCKER_STAGE=release \
+            SIM_DOCKER_CACHE_FROM="${DOCKER_REPO}:latest" \
+            build-sim-docker
       - name: "Install bentobox-sdk"
         run: |
           make install-sdk

.gitignore

@@ -341,3 +341,4 @@ sdk/ChangeLog
 sdk/AUTHORS
 sdk/bento/protos/
 sdk/docs/
+sdk/grpc/

.pre-commit-config.yaml

@@ -0,0 +1,17 @@
+repos:
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v2.3.0
+  hooks:
+    - id: end-of-file-fixer
+    - id: trailing-whitespace
+    - id: check-merge-conflict
+- repo: https://github.com/Yelp/detect-secrets
+  rev: v0.14.3
+  hooks:
+    - id: detect-secrets
+      args: ['--baseline', '.secrets.baseline']
+      exclude: package.lock.json
+- repo: https://github.com/psf/black
+  rev: 20.8b1
+  hooks:
+  -   id: black

.secrets.baseline

@@ -0,0 +1,67 @@
+{
+  "custom_plugin_paths": [],
+  "exclude": {
+    "files": null,
+    "lines": null
+  },
+  "generated_at": "2021-02-18T06:17:25Z",
+  "plugins_used": [
+    {
+      "name": "AWSKeyDetector"
+    },
+    {
+      "name": "ArtifactoryDetector"
+    },
+    {
+      "base64_limit": 4.5,
+      "name": "Base64HighEntropyString"
+    },
+    {
+      "name": "BasicAuthDetector"
+    },
+    {
+      "name": "CloudantDetector"
+    },
+    {
+      "hex_limit": 3,
+      "name": "HexHighEntropyString"
+    },
+    {
+      "name": "IbmCloudIamDetector"
+    },
+    {
+      "name": "IbmCosHmacDetector"
+    },
+    {
+      "name": "JwtTokenDetector"
+    },
+    {
+      "keyword_exclude": null,
+      "name": "KeywordDetector"
+    },
+    {
+      "name": "MailchimpDetector"
+    },
+    {
+      "name": "PrivateKeyDetector"
+    },
+    {
+      "name": "SlackDetector"
+    },
+    {
+      "name": "SoftlayerDetector"
+    },
+    {
+      "name": "StripeDetector"
+    },
+    {
+      "name": "TwilioKeyDetector"
+    }
+  ],
+  "results": {},
+  "version": "0.14.3",
+  "word_list": {
+    "file": null,
+    "hash": null
+  }
+}

infra/kustomize/engine/deployment.yaml

@@ -1,7 +1,7 @@
 #
 # bentobox engine
 # k8s deployment
-# 
+#
 
 apiVersion: apps/v1
 kind: Deployment

makefile

@@ -74,6 +74,9 @@ FIND_SIM_SRC:=$(FIND) $(SIM_SRC_DIRS) -type f \( -name "*.cpp" -o -name "*.h" \)
 SIM_BUILD_TYPE:=Debug
 SIM_DOCKER:=bentobox-sim
 SIM_DOCKER_STAGE:=release
+SIM_DOCKER_CACHE_FROM:=
+SIM_DOCKER_FLAGS:=$(if $(SIM_DOCKER_CACHE_FROM),--cache-from=$(SIM_DOCKER_CACHE_FROM),) \
+	--build-arg BUILDKIT_INLINE_CACHE=1
 SIM_PORT:=54242
 SIM_HOST:=0.0.0.0
 CMAKE_GENERATOR:=Ninja
@@ -91,7 +94,8 @@ build-sim: dep-sim
 		--target $(SIM_TARGET) --target $(SIM_TEST)
 
 build-sim-docker:
-	$(DOCKER) build --target $(SIM_DOCKER_STAGE) -t $(SIM_DOCKER) -f infra/docker/sim/Dockerfile .
+	$(DOCKER) build --target $(SIM_DOCKER_STAGE) $(SIM_DOCKER_FLAGS) \
+		-t $(SIM_DOCKER) -f infra/docker/sim/Dockerfile .
 
 test-sim: build-sim
 	$(SIM_BUILD_DIR)/$(SIM_TEST)
@@ -120,6 +124,7 @@ lint-sim: .clang-format
 SDK_SRC:=sdk
 PYTHON:=python
 BLACK_FMT:=python -m black
+MYPY:=python -m mypy
 PYTEST:=python -m pytest -vv
 PDOC:=python -m pdoc --force
 PIP=python -m pip
@@ -129,7 +134,7 @@ PIP=python -m pip
 dep-sdk-dev:
 	$(PIP) install -r $(SDK_SRC)/requirements-dev.txt
 
-build-sdk: dep-sdk-dev lint-sdk
+build-sdk: dep-sdk-dev
 	cd $(SDK_SRC) && $(PYTHON) setup.py sdist bdist_wheel
 
 format-sdk: dep-sdk-dev
@@ -139,8 +144,14 @@ format-sdk: dep-sdk-dev
 lint-sdk: dep-sdk-dev
 	$(BLACK_FMT) --check $(SDK_SRC)/bento
 	$(BLACK_FMT) --check $(SDK_SRC)/tests
-
-install-sdk:
+	# mypy requires the type stubs be generated for python protobuf bindings
+	$(FIND_PROTO_SRC) | xargs $(PROTOC) \
+		-I$(PROTO_SRC) \
+		--mypy_out=$(SDK_SRC) \
+		--mypy_grpc_out=$(SDK_SRC)
+	cd $(SDK_SRC) && $(MYPY) --config-file mypy.ini bento
+
+install-sdk: dep-sdk-dev
 	$(PIP) install -e $(SDK_SRC)
 
 test-sdk: dep-sdk-dev install-sdk

sdk/bento/client.py

@@ -45,8 +45,6 @@ def raise_native(err: RpcError):
         raise LookupError(err.details())
     elif status == StatusCode.ALREADY_EXISTS:
         raise FileExistsError(err.details())
-    elif status == StatusCode.OUT_OF_RANGE:
-        raise IndexError(err.details())
     else:
         raise RuntimeError(err.details())
 
@@ -143,7 +141,7 @@ def list_sims(self) -> List[str]:
             response = self.sim_grpc.ListSimulation(ListSimulationReq())
         except RpcError as e:
             raise_native(e)
-        return response.sim_names
+        return list(response.sim_names)
 
     def remove_sim(self, name: str):
         """Remove the simulation with the given name.

sdk/bento/ecs/base.py

@@ -4,7 +4,7 @@
 # Base classes
 #
 
-from typing import List, Any, Set, Union
+from typing import FrozenSet, List, Any, Set, Union
 from abc import ABC, abstractmethod, abstractproperty
 
 from bento.spec.ecs import ComponentDef
@@ -71,14 +71,14 @@ def get_component(self, name: str) -> Component:
         pass
 
     @abstractproperty
-    def id(self) -> int:  # type: ignore
+    def id(self) -> int:
         """Get the id of this Entity"""
-        pass
+        return 0  # placeholder to statisfy type checking
 
     @abstractproperty
-    def components(self) -> Set[Component]:  # type: ignore
+    def components(self) -> FrozenSet[Component]:
         """Get the components attached to this Entity"""
-        pass
+        return FrozenSet()  # placeholder to statisfy type checking
 
     def __getitem__(self, name: Union[str, ComponentDef]) -> Component:
         """Alias for get_component()"""

sdk/bento/ecs/grpc.py

@@ -5,7 +5,7 @@
 #
 
 
-from typing import Any, Set, List
+from typing import Any, FrozenSet, Set, List
 
 from bento.ecs import base
 from bento.types import Type
@@ -91,5 +91,5 @@ def id(self):
         return self.entity_id
 
     @property
-    def components(self) -> Set[Component]:
+    def components(self) -> FrozenSet[Component]:
         return frozenset(self.component_map.values())

sdk/bento/graph/analyzers.py

@@ -11,11 +11,10 @@
 from collections import deque
 from collections.abc import Iterable
 from math import inf
-from typing import List
 from gast import (
     AST,
     Constant,
-    List,
+    List as ListAST,
     Tuple,
     Pass,
     FunctionDef,
@@ -133,7 +132,7 @@ def analyze_assign(ast: AST) -> AST:
     assign_asts = [n for n in gast.walk(ast) if isinstance(n, assign_types)]
 
     for assign_ast in assign_asts:
-        iterable_types = (List, Tuple)
+        iterable_types = (ListAST, Tuple)
         assign_ast.is_unpack, assign_ast.is_multi = False, False
         if isinstance(assign_ast, Assign):
             # check if this unpacking assignment
@@ -191,7 +190,7 @@ def analyze_const(ast: AST) -> AST:
 
     # recursively walk AST to search for constants
     def walk_const(ast, part_of=None):
-        iterable_types = (List, Tuple)
+        iterable_types = (ListAST, Tuple)
         ignore_types = (Load, Store, Del)
 
         ast.is_constant = False

sdk/bento/graph/ast.py

@@ -247,7 +247,7 @@ def load_ast_module(ast: AST, remove_src: bool = True) -> Union[Any, Tuple[Any,
         # import the source as a module
         mod_spec = spec_from_file_location("compiled", f.name)
         module = module_from_spec(mod_spec)
-        mod_spec.loader.exec_module(module)
+        mod_spec.loader.exec_module(module)  # type: ignore
     # delete the temporary file manually as NamedTemporaryFile runs into
     # permission issues trying to remove it on Windows.
     if remove_src: