Merge pull request #148 from CSCfi/feature/type-hints

Feature/type hints

Author: blankdots

.github/workflows/publish.yml

@@ -13,37 +13,45 @@ jobs:
     steps:
       - uses: actions/checkout@v1
       - name: Login to DockerHub Registry
-        run: echo ${{ secrets.DOCKER_PASSWORD }} | docker login -u ${{ secrets.DOCKER_USERNAME }} --password-stdin
+        run: echo '${{ secrets.DOCKER_PASSWORD }}' | docker login -u '${{ secrets.DOCKER_USERNAME }}' --password-stdin
       - name: Get the version
         id: vars
         run: echo ::set-output name=tag::$(echo ${GITHUB_REF:10})
       - name: Build the tagged Docker image
+        if:  ${{ steps.vars.outputs.tag != '/master' }}
         run: docker build . --file Dockerfile --tag cscfi/beacon-python:${{steps.vars.outputs.tag}}
       - name: Push the tagged Docker image
+        if:  ${{ steps.vars.outputs.tag != '/master' }}
         run: docker push cscfi/beacon-python:${{steps.vars.outputs.tag}}
       - name: Build the latest Docker image
+        if:  ${{ steps.vars.outputs.tag == '/master' }}
         run: docker build . --file Dockerfile --tag cscfi/beacon-python:latest
       - name: Push the latest Docker image
+        if:  ${{ steps.vars.outputs.tag == '/master' }}
         run: docker push cscfi/beacon-python:latest
   push_data_to_registry:
     name: Push Dataloader Docker image to Docker Hub
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
       - name: Login to DockerHub Registry
-        run: echo ${{ secrets.DOCKER_PASSWORD }} | docker login -u ${{ secrets.DOCKER_USERNAME }} --password-stdin
+        run: echo '${{ secrets.DOCKER_PASSWORD }}' | docker login -u '${{ secrets.DOCKER_USERNAME }}' --password-stdin
       - name: Get the version
         id: vars
         run: echo ::set-output name=tag::$(echo ${GITHUB_REF:10})
       - name: Build the tagged Docker image
+        if:  ${{ steps.vars.outputs.tag != '/master' }}
         run: |
           pushd deploy/test
           docker build . --file Dockerfile --tag cscfi/beacon-dataloader:${{steps.vars.outputs.tag}}
       - name: Push the tagged Docker image
+        if:  ${{ steps.vars.outputs.tag != '/master' }}
         run: docker push cscfi/beacon-dataloader:${{steps.vars.outputs.tag}}
       - name: Build the latest Docker image
+        if:  ${{ steps.vars.outputs.tag == '/master' }}
         run: |
           pushd deploy/test
           docker build . --file Dockerfile --tag cscfi/beacon-dataloader:latest
       - name: Push the latest Docker image
-        run: docker push cscfi/beacon-dataloader:latest
+        if:  ${{ steps.vars.outputs.tag == '/master' }}
+        run: docker push cscfi/beacon-dataloader:latest

.github/workflows/style.yml

@@ -25,4 +25,8 @@ jobs:
     - name: Test flake8 syntax with tox
       run: tox -e flake8
     - name: Do bandit static check with tox
-      run: tox -e bandit
+      run: tox -e bandit
+    - name: Install libcurl-devel
+      run: sudo apt-get install libcurl4-openssl-dev
+    - name: Do typing check with tox
+      run: tox -e mypy

beacon_api/api/exceptions.py

@@ -4,13 +4,17 @@
 """
 
 import json
+from typing import Dict
 from aiohttp import web
 from .. import __apiVersion__
 from ..utils.logging import LOG
 from ..conf import CONFIG_INFO
 
 
-def process_exception_data(request, host, error_code, error):
+def process_exception_data(request: Dict,
+                           host: str,
+                           error_code: int,
+                           error: str) -> Dict:
     """Return request data as dictionary.
 
     Generates custom exception messages based on request parameters.
@@ -45,7 +49,8 @@ class BeaconBadRequest(web.HTTPBadRequest):
     Used in conjunction with JSON Schema validator.
     """
 
-    def __init__(self, request, host, error):
+    def __init__(self, request: Dict,
+                 host: str, error: str) -> None:
         """Return custom bad request exception."""
         data = process_exception_data(request, host, 400, error)
         super().__init__(text=json.dumps(data), content_type="application/json")
@@ -59,7 +64,8 @@ class BeaconUnauthorised(web.HTTPUnauthorized):
     Used in conjunction with Token authentication aiohttp middleware.
     """
 
-    def __init__(self, request, host, error, error_message):
+    def __init__(self, request: Dict,
+                 host: str, error: str, error_message: str) -> None:
         """Return custom unauthorized exception."""
         data = process_exception_data(request, host, 401, error)
         headers_401 = {"WWW-Authenticate": f"Bearer realm=\"{CONFIG_INFO.url}\"\n\
@@ -79,7 +85,8 @@ class BeaconForbidden(web.HTTPForbidden):
     but not granted the resource. Used in conjunction with Token authentication aiohttp middleware.
     """
 
-    def __init__(self, request, host, error):
+    def __init__(self, request: Dict,
+                 host: str, error: str) -> None:
         """Return custom forbidden exception."""
         data = process_exception_data(request, host, 403, error)
         super().__init__(content_type="application/json", text=json.dumps(data))
@@ -92,7 +99,7 @@ class BeaconServerError(web.HTTPInternalServerError):
     The 500 error is not specified by the Beacon API, thus as simple error would do.
     """
 
-    def __init__(self, error):
+    def __init__(self, error: str) -> None:
         """Return custom forbidden exception."""
         data = {'errorCode': 500,
                 'errorMessage': error}

beacon_api/api/info.py

@@ -6,6 +6,7 @@
 .. note:: See ``beacon_api`` root folder ``__init__.py`` for changing values used here.
 """
 
+from typing import Dict
 from .. import __apiVersion__, __title__, __version__, __description__, __url__, __alturl__, __handover_beacon__
 from .. import __createtime__, __updatetime__, __org_id__, __org_name__, __org_description__
 from .. import __org_address__, __org_logoUrl__, __org_welcomeUrl__, __org_info__, __org_contactUrl__
@@ -17,7 +18,7 @@
 
 
 @cached(ttl=60, key="ga4gh_info", serializer=JsonSerializer())
-async def ga4gh_info(host):
+async def ga4gh_info(host: str) -> Dict:
     """Construct the `Beacon` app information dict in GA4GH Discovery format.
 
     :return beacon_info: A dict that contain information about the ``Beacon`` endpoint.
@@ -43,7 +44,7 @@ async def ga4gh_info(host):
 
 
 @cached(ttl=60, key="info_key", serializer=JsonSerializer())
-async def beacon_info(host, pool):
+async def beacon_info(host: str, pool) -> Dict:
     """Construct the `Beacon` app information dict.
 
     :return beacon_info: A dict that contain information about the ``Beacon`` endpoint.

beacon_api/api/query.py

@@ -5,6 +5,7 @@
 start or end position.
 """
 
+from typing import Dict, Tuple, List, Optional
 from ..utils.logging import LOG
 from .. import __apiVersion__, __handover_beacon__, __handover_drs__
 from ..utils.data_query import filter_exists, find_datasets, fetch_datasets_access
@@ -13,7 +14,11 @@
 from .exceptions import BeaconUnauthorised, BeaconForbidden, BeaconBadRequest
 
 
-def access_resolution(request, token, host, public_data, registered_data, controlled_data):
+def access_resolution(request: Dict, token: Dict,
+                      host: str,
+                      public_data: List[str],
+                      registered_data: List[str],
+                      controlled_data: List[str]) -> Tuple[List[str], List[str]]:
     """Determine the access level for a user.
 
     Depends on user bona_fide_status, and by default it should be PUBLIC.
@@ -23,12 +28,12 @@ def access_resolution(request, token, host, public_data, registered_data, contro
     # unless the request is for specific datasets
     if public_data:
         permissions.append("PUBLIC")
-    access = set(public_data)  # empty if no datasets are given
+    accessible_datasets = set(public_data)  # empty if no datasets are given
 
     # for now we are expecting that the permissions are a list of datasets
     if registered_data and token["bona_fide_status"] is True:
         permissions.append("REGISTERED")
-        access = access.union(set(registered_data))
+        accessible_datasets = accessible_datasets.union(set(registered_data))
     # if user requests public datasets do not throw an error
     # if both registered and controlled datasets are request this will be shown first
     elif registered_data and not public_data:
@@ -43,7 +48,7 @@ def access_resolution(request, token, host, public_data, registered_data, contro
         # Default event, when user doesn't specify dataset ids
         # Contains only dataset ids from token that are present at beacon
         controlled_access = set(controlled_data).intersection(set(token['permissions']))
-        access = access.union(controlled_access)
+        accessible_datasets = accessible_datasets.union(controlled_access)
         if controlled_access:
             permissions.append("CONTROLLED")
     # if user requests public datasets do not throw an error
@@ -54,11 +59,12 @@ def access_resolution(request, token, host, public_data, registered_data, contro
             raise BeaconUnauthorised(request, host, "missing_token", 'Unauthorized access to dataset(s), missing token.')
         # token is present, but is missing perms (user authed but no access)
         raise BeaconForbidden(request, host, 'Access to dataset(s) is forbidden.')
-    LOG.info(f"Accesible datasets are: {list(access)}.")
-    return permissions, list(access)
 
+    LOG.info(f"Accesible datasets are: {list(accessible_datasets)}.")
+    return permissions, list(accessible_datasets)
 
-async def query_request_handler(params):
+
+async def query_request_handler(params: Tuple) -> Dict:
     """Handle the parameters of the query endpoint in order to find the required datasets.
 
     params = db_pool, method, request, token, host
@@ -91,18 +97,20 @@ async def query_request_handler(params):
         raise BeaconBadRequest(request, params[4], "endMin value Must be smaller than endMax value")
     if request.get("startMin") and request.get("startMin") > request.get("startMax"):
         raise BeaconBadRequest(request, params[4], "startMin value Must be smaller than startMax value")
-    requested_position = (request.get("start", None), request.get("end", None),
-                          request.get("startMin", None), request.get("startMax", None),
-                          request.get("endMin", None), request.get("endMax", None))
+    requested_position: Tuple[Optional[int], ...] = (request.get("start", None), request.get("end", None),
+                                                     request.get("startMin", None), request.get("startMax", None),
+                                                     request.get("endMin", None), request.get("endMax", None))
 
     # Get dataset ids that were requested, sort by access level
     # If request is empty (default case) the three dataset variables contain all datasets by access level
     # Datasets are further filtered using permissions from token
     public_datasets, registered_datasets, controlled_datasets = await fetch_datasets_access(params[0], request.get("datasetIds"))
-    access_type, accessible_datasets = access_resolution(request, params[3], params[4], public_datasets,
-                                                         registered_datasets, controlled_datasets)
+    access_type, accessible_datasets = access_resolution(request,
+                                                         params[3], params[4],
+                                                         public_datasets, registered_datasets, controlled_datasets)
     if 'mateName' in request or alleleRequest.get('variantType') == 'BND':
-        datasets = await find_fusion(params[0], request.get("assemblyId"), requested_position, request.get("referenceName"),
+        datasets = await find_fusion(params[0],
+                                     request.get("assemblyId"), requested_position, request.get("referenceName"),
                                      request.get("referenceBases"), request.get('mateName'),
                                      accessible_datasets, access_type, request.get("includeDatasetResponses", "NONE"))
     else:
@@ -122,4 +130,5 @@ async def query_request_handler(params):
 
     if __handover_drs__:
         beacon_response['beaconHandover'] = make_handover(__handover_beacon__, [x['datasetId'] for x in datasets])
+
     return beacon_response

beacon_api/app.py

@@ -28,7 +28,7 @@
 # ----------------------------------------------------------------------------------------------------------------------
 @routes.get('/')  # For Beacon API Specification
 @routes.get('/service-info')  # For GA4GH Discovery Specification
-async def beacon_get(request):
+async def beacon_get(request: web.Request) -> web.Response:
     """
     Use the HTTP protocol 'GET' to return a Json object of all the necessary info on the beacon and the API.
 
@@ -54,7 +54,7 @@ async def beacon_get(request):
 # These could be put under a @route.view('/query')
 @routes.get('/query')
 @validate(load_schema("query"))
-async def beacon_get_query(request):
+async def beacon_get_query(request: web.Request) -> web.Response:
     """Find datasets using GET endpoint."""
     method, processed_request = await parse_request_object(request)
     params = request.app['pool'], method, processed_request, request["token"], request.host
@@ -64,15 +64,15 @@ async def beacon_get_query(request):
 
 @routes.post('/query')
 @validate(load_schema("query"))
-async def beacon_post_query(request):
+async def beacon_post_query(request: web.Request) -> web.Response:
     """Find datasets using POST endpoint."""
     method, processed_request = await parse_request_object(request)
     params = request.app['pool'], method, processed_request, request["token"], request.host
     response = await query_request_handler(params)
     return web.json_response(response, content_type='application/json', dumps=json.dumps)
 
 
-async def initialize(app):
+async def initialize(app: web.Application) -> None:
     """Spin up DB a connection pool with the HTTP server."""
     # TO DO check if table and Database exist
     # and maybe exit gracefully or at least wait for a bit
@@ -81,7 +81,7 @@ async def initialize(app):
     set_cors(app)
 
 
-async def destroy(app):
+async def destroy(app: web.Application) -> None:
     """Upon server close, close the DB connection pool."""
     # will defer this to asyncpg
     await app['pool'].close()  # pragma: no cover
@@ -104,7 +104,7 @@ def set_cors(server):
         cors.add(route)
 
 
-async def init():
+async def init() -> web.Application:
     """Initialise server."""
     beacon = web.Application(middlewares=[token_auth()])
     beacon.router.add_routes(routes)
@@ -129,6 +129,6 @@ def main():
 
 if __name__ == '__main__':
     if sys.version_info < (3, 6):
-        LOG.error("beacon-python requires python3.6")
+        LOG.error("beacon-python requires python 3.6")
         sys.exit(1)
     main()

beacon_api/conf/__init__.py

@@ -1,22 +1,30 @@
 """Beacon Python Application Configuration."""
 
 import json
-import os
+from os import environ
 from configparser import ConfigParser
 from collections import namedtuple
 from distutils.util import strtobool
+from pathlib import Path
 
+from typing import Any, Dict, List, Union
 
-def parse_drspaths(paths):
+
+def convert(dictionary: Dict) -> tuple:
+    """Convert dictionary to Named tuple."""
+    return namedtuple('Config', dictionary.keys())(**dictionary)
+
+
+def parse_drspaths(paths: str) -> List[List[str]]:
     """Parse handover configuration."""
     return [p.strip().split(',', 2) for p in paths.split('\n') if p.split()]
 
 
-def parse_config_file(path):
+def parse_config_file(path) -> Any:
     """Parse configuration file."""
     config = ConfigParser()
     config.read(path)
-    config_vars = {
+    config_vars: Dict[str, Union[str, int, List[List[str]]]] = {
         'title': config.get('beacon_general_info', 'title'),
         'version': config.get('beacon_general_info', 'version'),
         'author': config.get('beacon_general_info', 'author'),
@@ -45,28 +53,28 @@ def parse_config_file(path):
         'org_logoUrl': config.get('organisation_info', 'org_logoUrl'),
         'org_info': config.get('organisation_info', 'org_info')
     }
-    return namedtuple("Config", config_vars.keys())(*config_vars.values())
+    return convert(config_vars)
 
 
-CONFIG_INFO = parse_config_file(os.environ.get('CONFIG_FILE', os.path.join(os.path.dirname(__file__), 'config.ini')))
+CONFIG_INFO = parse_config_file(environ.get('CONFIG_FILE', str(Path(__file__).resolve().parent.joinpath('config.ini'))))
 
 
-def parse_oauth2_config_file(path):
+def parse_oauth2_config_file(path: str) -> Any:
     """Parse configuration file."""
     config = ConfigParser()
     config.read(path)
-    config_vars = {
+    config_vars: Dict[str, Union[str, bool, None]] = {
         'server': config.get('oauth2', 'server'),
         'issuers': config.get('oauth2', 'issuers'),
         'userinfo': config.get('oauth2', 'userinfo'),
         'audience': config.get('oauth2', 'audience') or None,
         'verify_aud': bool(strtobool(config.get('oauth2', 'verify_aud'))),
         'bona_fide_value': config.get('oauth2', 'bona_fide_value')
     }
-    return namedtuple("Config", config_vars.keys())(*config_vars.values())
+    return convert(config_vars)
 
 
-OAUTH2_CONFIG = parse_oauth2_config_file(os.environ.get('CONFIG_FILE', os.path.join(os.path.dirname(__file__), 'config.ini')))
+OAUTH2_CONFIG = parse_oauth2_config_file(environ.get('CONFIG_FILE', str(Path(__file__).resolve().parent.joinpath('config.ini'))))
 # Sample query file should be of format [{BeaconAlleleRequest}] https://github.com/ga4gh-beacon/specification/
-sampleq_file = os.environ.get('SAMPLEQUERY_FILE', os.path.join(os.path.dirname(__file__), 'sample_queries.json'))
-SAMPLE_QUERIES = json.load(open(sampleq_file)) if os.path.isfile(sampleq_file) else []
+sampleq_file = Path(environ.get('SAMPLEQUERY_FILE', str(Path(__file__).resolve().parent.joinpath('sample_queries.json'))))
+SAMPLE_QUERIES = json.load(open(sampleq_file)) if sampleq_file.is_file() else []

beacon_api/conf/config.py

@@ -16,11 +16,12 @@
 
 import os
 import asyncpg
+from typing import Awaitable
 
 DB_SCHEMA = os.environ.get('DATABASE_SCHEMA', None)
 
 
-async def init_db_pool():
+async def init_db_pool() -> Awaitable:
     """Create a connection pool.
 
     As we will have frequent requests to the database it is recommended to create a connection pool.