Refactor logging and error messages in serverless (#1923)

* Add versioning to Docker image name Signed-off-by: Felix Wang <[email protected]> * Add more details to error messages Signed-off-by: Felix Wang <[email protected]> * Change versioning Signed-off-by: Felix Wang <[email protected]> * Switch from print statements to logger Signed-off-by: Felix Wang <[email protected]> * Change logging settings to be controlled through command line for Feast CLI Signed-off-by: Felix Wang <[email protected]> * Remove versioning changes for Docker image name Signed-off-by: Felix Wang <[email protected]> * Update logging Signed-off-by: Felix Wang <[email protected]> * Enforce default logging level at the argument level Signed-off-by: Felix Wang <[email protected]>
feast-dev · Oct 6, 2021 · a5b8c26 · a5b8c26
1 parent 7f35793
commit a5b8c26
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 34 deletions.
diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py
@@ -15,7 +15,7 @@
 import logging
 from datetime import datetime
 from pathlib import Path
-from typing import List
+from typing import List, Optional
 
 import click
 import pkg_resources
@@ -57,8 +57,13 @@ def format_options(self, ctx: click.Context, formatter: click.HelpFormatter):
     "-c",
     help="Switch to a different feature repository directory before executing the given subcommand.",
 )
+@click.option(
+    "--log-level",
+    default="info",
+    help="The logging level. One of DEBUG, INFO, WARNING, ERROR, and CRITICAL (case-insensitive).",
+)
 @click.pass_context
-def cli(ctx: click.Context, chdir: str):
+def cli(ctx: click.Context, chdir: Optional[str], log_level: str):
     """
     Feast CLI
 
@@ -68,6 +73,15 @@ def cli(ctx: click.Context, chdir: str):
     """
     ctx.ensure_object(dict)
     ctx.obj["CHDIR"] = Path.cwd() if chdir is None else Path(chdir).absolute()
+    try:
+        level = getattr(logging, log_level.upper())
+        logging.basicConfig(
+            format="%(asctime)s %(levelname)s:%(message)s",
+            datefmt="%m/%d/%Y %I:%M:%S %p",
+            level=level,
+        )
+    except Exception as e:
+        raise e
     pass
 
 

diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py
@@ -282,10 +282,14 @@ def __init__(self):
 
 
 class AwsLambdaDoesNotExist(Exception):
-    def __init__(self):
-        super().__init__("The created AWS Lambda function does not exist.")
+    def __init__(self, resource_name: str):
+        super().__init__(
+            f"The AWS Lambda function {resource_name} should have been created properly, but does not exist."
+        )
 
 
 class AwsAPIGatewayDoesNotExist(Exception):
-    def __init__(self):
-        super().__init__("The created AWS API Gateway does not exist.")
+    def __init__(self, resource_name: str):
+        super().__init__(
+            f"The AWS API Gateway {resource_name} should have been created properly, but does not exist."
+        )
diff --git a/sdk/python/feast/infra/aws.py b/sdk/python/feast/infra/aws.py
@@ -1,4 +1,5 @@
 import base64
+import logging
 import os
 import uuid
 from datetime import datetime
@@ -9,7 +10,6 @@
 
 from colorama import Fore, Style
 
-from feast import __version__
 from feast.constants import (
     AWS_LAMBDA_FEATURE_SERVER_IMAGE,
     FEAST_USAGE,
@@ -30,6 +30,7 @@
 from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto
 from feast.registry_store import RegistryStore
 from feast.repo_config import RegistryConfig
+from feast.version import get_version
 
 try:
     import boto3
@@ -38,11 +39,10 @@
 
     raise FeastExtrasDependencyImportError("aws", str(e))
 
+_logger = logging.getLogger(__name__)
 
-class AwsProvider(PassthroughProvider):
-    def _get_lambda_name(self, project: str):
-        return f"feast-python-server-{project}-{__version__.replace('+', '_').replace('.', '_')}"
 
+class AwsProvider(PassthroughProvider):
     def update_infra(
         self,
         project: str,
@@ -63,9 +63,8 @@ def update_infra(
 
         if self.repo_config.feature_server and self.repo_config.feature_server.enabled:
             image_uri = self._upload_docker_image(project)
-            print("Deploying feature server...")
+            _logger.info("Deploying feature server...")
 
-            assert self.repo_config.repo_path
             if not self.repo_config.repo_path:
                 raise RepoConfigPathDoesNotExist()
             with open(self.repo_config.repo_path / "feature_store.yaml", "rb") as f:
@@ -79,7 +78,7 @@ def update_infra(
 
             if function is None:
                 # If the Lambda function does not exist, create it.
-                print("  Creating AWS Lambda...")
+                _logger.info("  Creating AWS Lambda...")
                 lambda_client.create_function(
                     FunctionName=resource_name,
                     Role=self.repo_config.feature_server.execution_role_name,
@@ -95,14 +94,12 @@ def update_infra(
                     Tags={
                         "feast-owned": "True",
                         "project": project,
-                        "feast-sdk-version": __version__.replace("+", "_").replace(
-                            ".", "_"
-                        ),
+                        "feast-sdk-version": get_version(),
                     },
                 )
                 function = aws_utils.get_lambda_function(lambda_client, resource_name)
                 if not function:
-                    raise AwsLambdaDoesNotExist()
+                    raise AwsLambdaDoesNotExist(resource_name)
             else:
                 # If the feature_store.yaml has changed, need to update the environment variable.
                 env = function.get("Environment", {}).get("Variables", {})
@@ -111,7 +108,7 @@ def update_infra(
                     # It's expected that feature_store.yaml is not regularly updated while the lambda
                     # is serving production traffic. However, the update in registry (e.g. modifying
                     # feature views, feature services, and other definitions does not update lambda).
-                    print("  Updating AWS Lambda...")
+                    _logger.info("  Updating AWS Lambda...")
 
                     lambda_client.update_function_configuration(
                         FunctionName=resource_name,
@@ -123,7 +120,7 @@ def update_infra(
             api = aws_utils.get_first_api_gateway(api_gateway_client, resource_name)
             if not api:
                 # If the API Gateway doesn't exist, create it
-                print("  Creating AWS API Gateway...")
+                _logger.info("  Creating AWS API Gateway...")
                 api = api_gateway_client.create_api(
                     Name=resource_name,
                     ProtocolType="HTTP",
@@ -132,13 +129,11 @@ def update_infra(
                     Tags={
                         "feast-owned": "True",
                         "project": project,
-                        "feast-sdk-version": __version__.replace("+", "_").replace(
-                            ".", "_"
-                        ),
+                        "feast-sdk-version": get_version(),
                     },
                 )
                 if not api:
-                    raise AwsAPIGatewayDoesNotExist()
+                    raise AwsAPIGatewayDoesNotExist(resource_name)
                 # Make sure to give AWS Lambda a permission to be invoked by the newly created API Gateway
                 api_id = api["ApiId"]
                 region = lambda_client.meta.region_name
@@ -163,20 +158,20 @@ def teardown_infra(
             self.repo_config.feature_server is not None
             and self.repo_config.feature_server.enabled
         ):
-            print("Tearing down feature server...")
+            _logger.info("Tearing down feature server...")
             resource_name = self._get_lambda_name(project)
             lambda_client = boto3.client("lambda")
             api_gateway_client = boto3.client("apigatewayv2")
 
             function = aws_utils.get_lambda_function(lambda_client, resource_name)
 
             if function is not None:
-                print("  Tearing down AWS Lambda...")
+                _logger.info("  Tearing down AWS Lambda...")
                 aws_utils.delete_lambda_function(lambda_client, resource_name)
 
             api = aws_utils.get_first_api_gateway(api_gateway_client, resource_name)
             if api is not None:
-                print("  Tearing down AWS API Gateway...")
+                _logger.info("  Tearing down AWS API Gateway...")
                 aws_utils.delete_api_gateway(api_gateway_client, api["ApiId"])
 
     def _upload_docker_image(self, project: str) -> str:
@@ -213,16 +208,16 @@ def _upload_docker_image(self, project: str) -> str:
 
             raise DockerDaemonNotRunning()
 
-        print(
+        _logger.info(
             f"Pulling remote image {Style.BRIGHT + Fore.GREEN}{AWS_LAMBDA_FEATURE_SERVER_IMAGE}{Style.RESET_ALL}:"
         )
         docker_client.images.pull(AWS_LAMBDA_FEATURE_SERVER_IMAGE)
 
-        version = __version__.replace("+", "_").replace(".", "_")
+        version = get_version()
         repository_name = f"feast-python-server-{project}-{version}"
         ecr_client = boto3.client("ecr")
         try:
-            print(
+            _logger.info(
                 f"Creating remote ECR repository {Style.BRIGHT + Fore.GREEN}{repository_name}{Style.RESET_ALL}:"
             )
             response = ecr_client.create_repository(repositoryName=repository_name)
@@ -243,13 +238,16 @@ def _upload_docker_image(self, project: str) -> str:
 
         image = docker_client.images.get(AWS_LAMBDA_FEATURE_SERVER_IMAGE)
         image_remote_name = f"{repository_uri}:{version}"
-        print(
+        _logger.info(
             f"Pushing local image to remote {Style.BRIGHT + Fore.GREEN}{image_remote_name}{Style.RESET_ALL}:"
         )
         image.tag(image_remote_name)
         docker_client.api.push(repository_uri, tag=version)
         return image_remote_name
 
+    def _get_lambda_name(self, project: str):
+        return f"feast-python-server-{project}-{get_version()}"
+
 
 class S3RegistryStore(RegistryStore):
     def __init__(self, registry_config: RegistryConfig, repo_path: Path):

diff --git a/sdk/python/feast/version.py b/sdk/python/feast/version.py
@@ -2,10 +2,7 @@
 
 
 def get_version():
-    """
-    Returns version information of the Feast Python Package
-    """
-
+    """Returns version information of the Feast Python Package."""
     try:
         sdk_version = pkg_resources.get_distribution("feast").version
     except pkg_resources.DistributionNotFound: