From a5b8c267e558cd88cb20339d23a89750ffcc9136 Mon Sep 17 00:00:00 2001 From: Felix Wang Date: Wed, 6 Oct 2021 16:33:28 -0700 Subject: [PATCH] Refactor logging and error messages in serverless (#1923) * Add versioning to Docker image name Signed-off-by: Felix Wang * Add more details to error messages Signed-off-by: Felix Wang * Change versioning Signed-off-by: Felix Wang * Switch from print statements to logger Signed-off-by: Felix Wang * Change logging settings to be controlled through command line for Feast CLI Signed-off-by: Felix Wang * Remove versioning changes for Docker image name Signed-off-by: Felix Wang * Update logging Signed-off-by: Felix Wang * Enforce default logging level at the argument level Signed-off-by: Felix Wang --- sdk/python/feast/cli.py | 18 ++++++++++++-- sdk/python/feast/errors.py | 12 ++++++--- sdk/python/feast/infra/aws.py | 46 +++++++++++++++++------------------ sdk/python/feast/version.py | 5 +--- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 12be1e356d..c63975cc3a 100644 --- 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 index d4a25a7abe..a973730f89 100644 --- 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 index 94cf8fd30f..652a3ef798 100644 --- 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,7 +158,7 @@ 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") @@ -171,12 +166,12 @@ def teardown_infra( 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 index 78ec6e1a31..cc5190e339 100644 --- 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: