refactor(robot-server): start pulling routers to top-level modules

CONTRIBUTING.md

@@ -238,7 +238,7 @@ To help with code quality and maintainability, we use a collection of tools that
 
 - [Linters][lint]
   - Analyze the code for various potential bugs and errors
-  - [Pylama][pylama] - Python code audit tool
+  - [Flake8][flake8] - Python code audit tool
   - [ESLint][eslint] - JavaScript/JSON linter
   - [stylelint][] - CSS linter
 - [Typecheckers][type-check]
@@ -274,7 +274,7 @@ make format
 [type-safe]: https://en.wikipedia.org/wiki/Type_safety
 [code-style]: https://en.wikipedia.org/wiki/Programming_style
 [eslint]: https://eslint.org/
-[pylama]: https://github.com/klen/pylama
+[flake8]: https://flake8.pycqa.org
 [stylelint]: https://stylelint.io/
 [flow]: https://flow.org/
 [mypy]: http://mypy-lang.org/

robot-server/.flake8

@@ -29,5 +29,10 @@ per-file-ignores =
     robot_server/settings.py:ANN,D
     robot_server/robot/*:ANN,D
     robot_server/service/*:ANN,D
-    robot_server/system/*:ANN,D
-    tests/*:ANN,D
+    tests/integration/*:ANN,D
+    tests/robot/*:ANN,D
+    tests/service/*:ANN,D
+    tests/conftest.py:ANN,D
+    tests/test_app.py:ANN,D
+    tests/test_hardware_wrapper.py:ANN,D
+    tests/test_util.py:ANN,D

robot-server/Makefile

@@ -74,13 +74,13 @@ test:
 
 .PHONY: lint
 lint: $(ot_py_sources)
-	$(python) -m mypy $(SRC_PATH)
+	$(python) -m mypy $(SRC_PATH) tests/health tests/system
 	$(python) -m flake8 $(SRC_PATH) tests setup.py
 
 .PHONY: dev
 dev: export OT_ROBOT_SERVER_DOT_ENV_PATH ?= dev.env
 dev:
-	$(pipenv) run uvicorn "robot_server.service.app:app" --host localhost --port 31950 --ws wsproto --reload
+	$(pipenv) run uvicorn "robot_server:app" --host localhost --port 31950 --ws wsproto --reload
 
 .PHONY: local-shell
 local-shell:

robot-server/README.rst

@@ -49,6 +49,6 @@ Our tests live in ``tests/robot_server`` and are run with `pytest <https://docs.
 
 Tests should be organized similarly to the organization of the module itself.
 
-We use `PyLama <https://github.com/klen/pylama>`_ for lint checks, and `mypy <http://mypy-lang.org/>`_ for type-checking annotations. Both of these tools are run in the ``lint`` makefile target, and is run in CI; PRs will not be merged with failing lint. Usage of ``noqa`` to temporarily disable lint is discouraged, but if you need to please disable only a specific rule and leave a comment explaining exactly why. The same goes with ``type: ignore``.
+We use `Flake8 <https://flake8.pycqa.org>`_ for lint checks, and `mypy <http://mypy-lang.org/>`_ for type-checking annotations. Both of these tools are run in the ``lint`` makefile target, and is run in CI; PRs will not be merged with failing lint. Usage of ``noqa`` to temporarily disable lint is discouraged, but if you need to please disable only a specific rule and leave a comment explaining exactly why. The same goes with ``type: ignore``.
 
 New code should have appropriate type annotations, and refactors of old code should try to add type annotations. We’re flexible about the refactor part, though - if adding type annotations greatly expands the scope of a PR, it’s OK to not add them as long as you explain this in the PR message.

robot-server/opentrons-robot-server.service

@@ -6,7 +6,7 @@ After=opentrons-status-leds.service
 
 [Service]
 Type=notify
-ExecStart=uvicorn robot_server.service.app:app --uds /run/aiohttp.sock --ws wsproto
+ExecStart=uvicorn robot_server:app --uds /run/aiohttp.sock --ws wsproto
 # Stop the button blinking
 ExecStartPost=systemctl stop opentrons-gpio-setup.service
 Environment=OT_SMOOTHIE_ID=AMA RUNNING_ON_PI=true

robot-server/robot_server/__init__.py

@@ -1 +1,10 @@
-"""Opentrons Robot HTTP API Server."""
+"""Opentrons Robot HTTP API Server.
+
+This server provides the main control interface for an Opentrons robot.
+"""
+
+from .app import app
+
+__all__ = [
+    "app",
+]

robot-server/robot_server/app.py

@@ -0,0 +1,195 @@
+"""Main FastAPI application."""
+import logging
+import traceback
+
+from opentrons import __version__
+from fastapi import FastAPI, Depends
+from fastapi.middleware.cors import CORSMiddleware
+from fastapi.exceptions import RequestValidationError
+from starlette.responses import Response, JSONResponse
+from starlette.requests import Request
+from starlette.exceptions import HTTPException as StarletteHTTPException
+from starlette.status import HTTP_422_UNPROCESSABLE_ENTITY
+from starlette.middleware.base import RequestResponseEndpoint
+
+from .service.dependencies import (
+    check_version_header,
+    get_rpc_server,
+    get_protocol_manager,
+    get_hardware_wrapper,
+    get_session_manager,
+)
+
+from .router import router
+
+from .service import initialize_logging
+from .service.legacy.models import V1BasicResponse
+
+from .service.errors import (
+    V1HandlerError,
+    transform_http_exception_to_json_api_errors,
+    transform_validation_error_to_json_api_errors,
+    consolidate_fastapi_response,
+    BaseRobotServerError,
+    ErrorResponse,
+    build_unhandled_exception_response,
+)
+
+from . import constants
+
+log = logging.getLogger(__name__)
+
+
+app = FastAPI(
+    title="Opentrons OT-2 HTTP API Spec",
+    description=(
+        "This OpenAPI spec describes the HTTP API of the Opentrons "
+        "OT-2. It may be retrieved from a robot on port 31950 at "
+        "/openapi. Some schemas used in requests and responses use "
+        "the `x-patternProperties` key to mean the JSON Schema "
+        "`patternProperties` behavior."
+    ),
+    version=__version__,
+)
+
+# cors
+app.add_middleware(
+    CORSMiddleware,
+    allow_origins=("*"),
+    allow_credentials=True,
+    allow_methods=["*"],
+    allow_headers=["*"],
+)
+
+# main router
+app.include_router(
+    router=router,
+    dependencies=[Depends(check_version_header)],
+)
+
+
+@app.on_event("startup")
+async def on_startup() -> None:
+    """Handle app startup."""
+    initialize_logging()
+    # Initialize api
+    (await get_hardware_wrapper()).async_initialize()
+
+
+@app.on_event("shutdown")
+async def on_shutdown() -> None:
+    """Handle app shutdown."""
+    s = await get_rpc_server()
+    await s.on_shutdown()
+    # Remove all sessions
+    await (await get_session_manager()).remove_all()
+    # Remove all uploaded protocols
+    (await get_protocol_manager()).remove_all()
+
+
+@app.middleware("http")
+async def api_version_response_header(
+    request: Request,
+    call_next: RequestResponseEndpoint
+) -> Response:
+    """Attach Opentrons-Version headers to responses."""
+    # Attach the version the request state. Optional dependency
+    #  check_version_header will override this value if check passes.
+    request.state.api_version = constants.API_VERSION
+
+    response: Response = await call_next(request)
+
+    # Put the api version in the response header
+    response.headers[constants.API_VERSION_HEADER] = str(request.state.api_version)
+    response.headers[constants.MIN_API_VERSION_HEADER] = str(constants.MIN_API_VERSION)
+    return response
+
+
+@app.exception_handler(BaseRobotServerError)
+async def robot_server_exception_handler(
+    request: Request, exc: BaseRobotServerError
+) -> JSONResponse:
+    """Catch robot server exceptions."""
+    if not exc.error.status:
+        exc.error.status = str(exc.status_code)
+    log.error(f"RobotServerError: {exc.error}")
+    return JSONResponse(
+        status_code=exc.status_code,
+        content=ErrorResponse(errors=[exc.error]).dict(
+            exclude_unset=True, exclude_none=True
+        ),
+    )
+
+
+@app.exception_handler(V1HandlerError)
+async def v1_exception_handler(request: Request, exc: V1HandlerError) -> JSONResponse:
+    """Catch legacy errors."""
+    log.error(f"V1HandlerError: {exc.status_code}: {exc.message}")
+    return JSONResponse(
+        status_code=exc.status_code, content=V1BasicResponse(message=exc.message).dict()
+    )
+
+
+@app.exception_handler(RequestValidationError)
+async def custom_request_validation_exception_handler(
+    request: Request, exception: RequestValidationError
+) -> JSONResponse:
+    """Custom handling of FastAPI request validation errors."""
+    log.error(f"{request.method} {request.url.path} : {str(exception)}")
+
+    if route_has_tag(request, constants.V1_TAG):
+        response = V1BasicResponse(
+            message=consolidate_fastapi_response(exception.errors())
+        ).dict()
+    else:
+        response = transform_validation_error_to_json_api_errors(
+            HTTP_422_UNPROCESSABLE_ENTITY, exception
+        ).dict(exclude_unset=True)
+
+    return JSONResponse(status_code=HTTP_422_UNPROCESSABLE_ENTITY, content=response)
+
+
+@app.exception_handler(StarletteHTTPException)
+async def custom_http_exception_handler(
+    request: Request, exception: StarletteHTTPException
+) -> JSONResponse:
+    """Handle HTTP exceptions."""
+    log.error(
+        f"{request.method} {request.url.path} : "
+        f"{exception.status_code}, {exception.detail}"
+    )
+
+    if route_has_tag(request, constants.V1_TAG):
+        response = V1BasicResponse(message=exception.detail).dict()
+    else:
+        response = transform_http_exception_to_json_api_errors(exception).dict(
+            exclude_unset=True
+        )
+
+    return JSONResponse(
+        status_code=exception.status_code,
+        content=response,
+    )
+
+
+@app.exception_handler(Exception)
+async def unexpected_exception_handler(
+    request: Request, exc: Exception
+) -> JSONResponse:
+    """Log unhandled errors (re-raise always)."""
+    log.error(f"Unhandled exception: {traceback.format_exc()}")
+    return JSONResponse(
+        status_code=500,
+        content=build_unhandled_exception_response(exc).dict(exclude_unset=True),
+    )
+
+
+def route_has_tag(request: Request, tag: str) -> bool:
+    """Check if router handling the request has the tag."""
+    router = request.scope.get("router")
+    if router:
+        for route in router.routes:
+            if route.endpoint == request.scope.get("endpoint"):
+                return tag in route.tags
+
+    return False

robot-server/robot_server/constants.py

@@ -1,27 +1,43 @@
 """Global constants for the HTTP API server."""
 
-# This is the version of the HTTP API used by the server.  This value should
-# be incremented anytime the schema of a request or response is changed. This
-# value is different and separate from the application version.
 from typing_extensions import Literal, Final
 
 API_VERSION: Final = 2
+"""The current version of the HTTP API used by the server.
+
+This value will be incremented any time the schema of a request or response
+is changed. The value is separate from the overall application version.
+"""
+
 
-# Minimum API version supported by the server. Increasing this value should
-# be considered a **breaking change** in the application.
 MIN_API_VERSION: Final = 2
+"""The minimum HTTP API version supported by the server.
+
+Incrementing this value would be considered a breaking change to the overall
+application, and would result in a major version bump of the software.
+"""
 
-# Keyword header value for a client to ask to use the latest HTTP API version.
-API_VERSION_LATEST_TYPE = Literal["*"]
-API_VERSION_LATEST: API_VERSION_LATEST_TYPE = "*"
 
-# Header identifying maximum acceptable version in request and actual version
-# used to create response. Mandatory in requests and responses.
 API_VERSION_HEADER: Final = "Opentrons-Version"
+"""Custom header to specify which HTTP API version is being requested or served.
+
+Mandatory in requests and response. Can be used by the server and clients to
+negotiate and migrate requests and responses to a version both parties understand.
+"""
+
 
-# Response header specifing minimum acceptable API version
 MIN_API_VERSION_HEADER: Final = "Opentrons-Min-Version"
+"""Header to specify the server's minumum supported HTTP API version.
+
+Mandatory in all responses, not used in requests.
+"""
+
+
+API_VERSION_LATEST_TYPE = Literal["*"]
+
+API_VERSION_LATEST: API_VERSION_LATEST_TYPE = "*"
+"""Version head value meaning 'give me the latest avilable version'"""
 
 
-# Tag applied to legacy api endpoints
 V1_TAG = "v1"
+"""Tag applied to legacy endpoints that are still supported in HTTP API v2."""

robot-server/robot_server/hardware_wrapper.py

@@ -1,27 +1,29 @@
 """Hardware interface wrapping module."""
 import asyncio
-import typing
+import logging
 from pathlib import Path
+from typing import Optional
 
-from notify_server.clients.publisher import Publisher
 from opentrons import ThreadManager, initialize as initialize_api
 from opentrons.hardware_control.simulator_setup import load_simulator
 from opentrons.hardware_control.types import HardwareEvent, HardwareEventType
 from opentrons.util.helpers import utc_now
 
-from robot_server.main import log
-from robot_server.settings import get_settings
-
+from notify_server.clients.publisher import Publisher
 from notify_server.models import event, topics
 from notify_server.models.hardware_event import DoorStatePayload
 
+from .settings import get_settings
+
+log = logging.getLogger(__name__)
+
 
 class HardwareWrapper:
     """Wrapper to support initializing the Opentrons api hardware controller."""
 
     def __init__(self, event_publisher: Publisher) -> None:
         """Initialize a HardwareWrapper."""
-        self._tm: typing.Optional[ThreadManager] = None
+        self._tm: Optional[ThreadManager] = None
         self._hardware_event_watcher = None
         self._event_publisher = event_publisher
 
@@ -79,7 +81,6 @@ def async_initialize(self) -> None:
         """Create task to initialize hardware."""
         asyncio.create_task(self.initialize())
 
-    # TODO(mc, 2021-04-12): fix up typing
-    def get_hardware(self):  # noqa: ANN201
-        """Get the wrapped ThreadManager."""
+    def get_hardware(self) -> Optional[ThreadManager]:
+        """Get the wrapped ThreadManager, if it exists."""
         return self._tm

robot-server/robot_server/health/__init__.py

@@ -0,0 +1,4 @@
+"""Health routes and models."""
+from .router import health_router
+
+__all__ = ["health_router"]