Remove the need to pass logger to APIs

The API handling code now retrieves a logger from `flask.current_app`
only when needed.

This exposed a small problem with the test environment where the `Flask`
app context was not setup for the unit tests.

Author: portante

lib/pbench/server/api/__init__.py

@@ -49,134 +49,132 @@ def register_endpoints(api, app, config):
     to make the APIs active."""
 
     base_uri = config.rest_uri
-    logger = app.logger
 
-    logger.info("Registering service endpoints with base URI {}", base_uri)
+    app.logger.info("Registering service endpoints with base URI {}", base_uri)
 
     api.add_resource(
         DatasetsContents,
         f"{base_uri}/datasets/contents/<string:dataset>/",
         f"{base_uri}/datasets/contents/<string:dataset>/<path:target>",
         endpoint="datasets_contents",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsDateRange,
         f"{base_uri}/datasets/daterange",
         endpoint="datasets_daterange",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsDelete,
         f"{base_uri}/datasets/delete/<string:dataset>",
         endpoint="datasets_delete",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsDetail,
         f"{base_uri}/datasets/detail/<string:dataset>",
         endpoint="datasets_detail",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsList,
         f"{base_uri}/datasets/list",
         endpoint="datasets_list",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsMappings,
         f"{base_uri}/datasets/mappings/<string:dataset_view>",
         endpoint="datasets_mappings",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsMetadata,
         f"{base_uri}/datasets/metadata/<string:dataset>",
         endpoint="datasets_metadata",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsInventory,
         f"{base_uri}/datasets/inventory/<string:dataset>",
         f"{base_uri}/datasets/inventory/<string:dataset>/",
         f"{base_uri}/datasets/inventory/<string:dataset>/<path:target>",
         endpoint="datasets_inventory",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         SampleNamespace,
         f"{base_uri}/datasets/namespace/<string:dataset>/<string:dataset_view>",
         endpoint="datasets_namespace",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         SampleValues,
         f"{base_uri}/datasets/values/<string:dataset>/<string:dataset_view>",
         endpoint="datasets_values",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsPublish,
         f"{base_uri}/datasets/publish/<string:dataset>",
         endpoint="datasets_publish",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         DatasetsSearch,
         f"{base_uri}/datasets/search",
         endpoint="datasets_search",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         EndpointConfig,
         f"{base_uri}/endpoints",
         endpoint="endpoints",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         Login,
         f"{base_uri}/login",
         endpoint="login",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         Logout,
         f"{base_uri}/logout",
         endpoint="logout",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         RegisterUser,
         f"{base_uri}/register",
         endpoint="register",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         ServerAudit,
         f"{base_uri}/server/audit",
         endpoint="server_audit",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         ServerConfiguration,
         f"{base_uri}/server/configuration",
         f"{base_uri}/server/configuration/",
         f"{base_uri}/server/configuration/<string:key>",
         endpoint="server_configuration",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
     api.add_resource(
         UserAPI,
         f"{base_uri}/user/<string:target_username>",
         endpoint="user",
-        resource_class_args=(logger,),
     )
     api.add_resource(
         Upload,
         f"{base_uri}/upload/<string:filename>",
         endpoint="upload",
-        resource_class_args=(config, logger),
+        resource_class_args=(config,),
     )
 
 

lib/pbench/server/api/resources/__init__.py

@@ -3,12 +3,11 @@
 from http import HTTPStatus
 import json
 from json.decoder import JSONDecodeError
-from logging import Logger
 from typing import Any, Callable, Dict, List, NamedTuple, Optional, Union
 import uuid
 
 from dateutil import parser as date_parser
-from flask import request
+from flask import current_app, request
 from flask.wrappers import Request, Response
 from flask_restful import abort, Resource
 from sqlalchemy.orm.query import Query
@@ -1111,7 +1110,6 @@ class ApiBase(Resource):
     def __init__(
         self,
         config: PbenchServerConfig,
-        logger: Logger,
         *schemas: ApiSchema,
         always_enabled: bool = False,
     ):
@@ -1130,7 +1128,6 @@ def __init__(
         """
         super().__init__()
         self.config = config
-        self.logger = logger
         self.schemas = ApiSchemaSet(*schemas)
         self.always_enabled = always_enabled
 
@@ -1222,14 +1219,14 @@ def _check_authorization(self, mode: ApiAuthorization):
             if user:
                 username = user.username
             else:
-                self.logger.error("User ID {} not found", user_id)
+                current_app.logger.error("User ID {} not found", user_id)
 
         # The ADMIN authorization doesn't involve a target resource owner or
         # access, so take care of that first as a special case. If there is
         # an authenticated user, and that user holds ADMIN access rights, the
         # check passes. Otherwise raise an "admin access" failure.
         if mode.type == ApiAuthorizationType.ADMIN:
-            self.logger.debug(
+            current_app.logger.debug(
                 "Authorizing {} access for {} to an administrative resource",
                 role,
                 authorized_user,
@@ -1244,7 +1241,7 @@ def _check_authorization(self, mode: ApiAuthorization):
 
         access = mode.access
 
-        self.logger.debug(
+        current_app.logger.debug(
             "Authorizing {} access for {} to user {} ({}) with access {} using {}",
             role,
             authorized_user,
@@ -1274,7 +1271,7 @@ def _check_authorization(self, mode: ApiAuthorization):
             if authorized_user is None:
                 # An unauthenticated user is never allowed to access private
                 # data nor to perform an potential mutation of data: REJECT
-                self.logger.warning(
+                current_app.logger.warning(
                     "Attempt to {} user {} data without login", role, username
                 )
                 raise UnauthorizedAccess(
@@ -1287,7 +1284,7 @@ def _check_authorization(self, mode: ApiAuthorization):
             elif role != OperationCode.READ and user_id is None:
                 # No target user is specified, so we won't allow mutation of
                 # data: REJECT
-                self.logger.warning(
+                current_app.logger.warning(
                     "Unauthorized attempt by {} to {} data with defaulted user",
                     authorized_user,
                     role,
@@ -1303,7 +1300,7 @@ def _check_authorization(self, mode: ApiAuthorization):
                 # We are mutating data, or reading private data, so the
                 # authenticated user must either be the owner of the data or
                 # must have ADMIN role: REJECT
-                self.logger.warning(
+                current_app.logger.warning(
                     "Unauthorized attempt by {} to {} user {} data",
                     authorized_user,
                     role,
@@ -1391,7 +1388,7 @@ def _build_sql_query(
         is_admin = authorized_user.is_admin() if authorized_user else False
         query = base_query
 
-        self.logger.debug(
+        current_app.logger.debug(
             "QUERY auth ID {}, user {!r}, access {!r}, admin {}",
             authorized_id,
             owner_id,
@@ -1422,24 +1419,28 @@ def _build_sql_query(
             owner_id and owner_id != authorized_id and not is_admin
         ):
             query = query.filter(Dataset.access == Dataset.PUBLIC_ACCESS)
-            self.logger.debug("QUERY: not self public")
+            current_app.logger.debug("QUERY: not self public")
         elif access:
             query = query.filter(Dataset.access == access)
             if not owner_id and access == Dataset.PRIVATE_ACCESS and not is_admin:
                 query = query.filter(Dataset.owner_id == authorized_id)
                 user_term = True
-            self.logger.debug("QUERY: user: {}, access: {}", authorized_id, access)
+            current_app.logger.debug(
+                "QUERY: user: {}, access: {}", authorized_id, access
+            )
         elif not owner_id and not is_admin:
             query = query.filter(
                 (Dataset.owner_id == authorized_id)
                 | (Dataset.access == Dataset.PUBLIC_ACCESS)
             )
             user_term = True
-            self.logger.debug("QUERY: self ({}) + public", authorized_user.username)
+            current_app.logger.debug(
+                "QUERY: self ({}) + public", authorized_user.username
+            )
         else:
             # Either "user" was specified and will be added to the filter,
             # or client is ADMIN and no access restrictions are required.
-            self.logger.debug(
+            current_app.logger.debug(
                 "QUERY: default, user: {}", owner_id if owner_id else authorized_user
             )
 
@@ -1513,7 +1514,7 @@ def _dispatch(
 
         api_name = self.__class__.__name__
 
-        self.logger.info("In {} {}: mime {}", method, api_name, request.mimetype)
+        current_app.logger.info("In {} {}: mime {}", method, api_name, request.mimetype)
 
         if method is ApiMethod.GET:
             execute = self._get
@@ -1576,15 +1577,15 @@ def _dispatch(
                 ApiParams(body=body_params, query=query_params, uri=uri_params),
             )
         except APIInternalError as e:
-            self.logger.exception("{} {}", api_name, e.details)
+            current_app.logger.exception("{} {}", api_name, e.details)
             abort(e.http_status, message=str(e))
         except APIAbort as e:
             abort(e.http_status, message=str(e))
         except Exception:
             # Construct an APIInternalError to get the UUID and standard return
             # message.
             x = APIInternalError("Unexpected validation exception")
-            self.logger.exception("{} {}", api_name, x.details)
+            current_app.logger.exception("{} {}", api_name, x.details)
             abort(x.http_status, message=str(x))
 
         # Automatically authorize the operation only if the API schema for the
@@ -1598,16 +1599,16 @@ def _dispatch(
             try:
                 self._check_authorization(auth_params)
             except UnauthorizedAccess as e:
-                self.logger.warning("{}: {}", api_name, e)
+                current_app.logger.warning("{}: {}", api_name, e)
                 abort(e.http_status, message=str(e))
             except APIInternalError as e:
-                self.logger.exception("{} {}", api_name, e.details)
+                current_app.logger.exception("{} {}", api_name, e.details)
                 abort(e.http_status, message=str(e))
             except Exception:
                 # Construct an APIInternalError to get the UUID and standard return
                 # message.
                 x = APIInternalError("Unexpected authorize exception")
-                self.logger.exception("{} {}", api_name, x.details)
+                current_app.logger.exception("{} {}", api_name, x.details)
                 abort(x.http_status, message=str(x))
 
         audit = None
@@ -1653,7 +1654,7 @@ def _dispatch(
                 )
             return response
         except APIInternalError as e:
-            self.logger.exception("{} {}", api_name, e.details)
+            current_app.logger.exception("{} {}", api_name, e.details)
             abort(e.http_status, message=str(e))
         except APIAbort as e:
             if auditing["finalize"]:
@@ -1666,12 +1667,12 @@ def _dispatch(
                         attributes=attr,
                     )
                 except Exception:
-                    self.logger.error(
+                    current_app.logger.error(
                         "Unexpected exception on audit: {}", json.dumps(auditing)
                     )
             abort(e.http_status, message=str(e))
         except Exception as e:
-            self.logger.exception(
+            current_app.logger.exception(
                 "Exception {} API error: {}: {!r}", api_name, e, json.dumps(auditing)
             )
             if auditing["finalize"]:

lib/pbench/server/api/resources/datasets_daterange.py

@@ -1,5 +1,3 @@
-from logging import Logger
-
 from flask.json import jsonify
 from flask.wrappers import Request, Response
 from sqlalchemy import func
@@ -25,10 +23,9 @@ class DatasetsDateRange(ApiBase):
     API class to retrieve the available date range of accessible datasets.
     """
 
-    def __init__(self, config: PbenchServerConfig, logger: Logger):
+    def __init__(self, config: PbenchServerConfig):
         super().__init__(
             config,
-            logger,
             ApiSchema(
                 ApiMethod.GET,
                 OperationCode.READ,

lib/pbench/server/api/resources/datasets_inventory.py

@@ -1,8 +1,7 @@
 from http import HTTPStatus
-from logging import Logger
 from urllib.request import Request
 
-from flask import send_file
+from flask import current_app, send_file
 from flask.wrappers import Response
 
 from pbench.server import OperationCode, PbenchServerConfig
@@ -26,10 +25,9 @@ class DatasetsInventory(ApiBase):
     API class to retrieve inventory files from a dataset
     """
 
-    def __init__(self, config: PbenchServerConfig, logger: Logger):
+    def __init__(self, config: PbenchServerConfig):
         super().__init__(
             config,
-            logger,
             ApiSchema(
                 ApiMethod.GET,
                 OperationCode.READ,
@@ -62,7 +60,7 @@ def _get(
         dataset = params.uri["dataset"]
         target = params.uri.get("target")
 
-        cache_m = CacheManager(self.config, self.logger)
+        cache_m = CacheManager(self.config, current_app.logger)
         try:
             tarball = cache_m.find_dataset(dataset.resource_id)
         except TarballNotFound as e: