From 5e753e4f72898f3723b5f69e11c2c968744e0815 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli <86618610+dmartinol@users.noreply.github.com> Date: Thu, 29 Aug 2024 09:39:17 +0200 Subject: [PATCH] refactor: Introduced base class FeastError for all Feast exceptions (#4465) introduced base class FeastError for all Feast exceptions, with initial methods to map the grpc and HTTP status code Signed-off-by: Daniele Martinoli --- sdk/python/feast/cli_utils.py | 2 +- sdk/python/feast/errors.py | 125 +++++++++++------- sdk/python/feast/permissions/enforcer.py | 9 +- .../feast/permissions/security_manager.py | 8 +- .../tests/unit/permissions/test_decorator.py | 6 +- .../unit/permissions/test_security_manager.py | 10 +- 6 files changed, 96 insertions(+), 64 deletions(-) diff --git a/sdk/python/feast/cli_utils.py b/sdk/python/feast/cli_utils.py index edfdab93e3..264a633c31 100644 --- a/sdk/python/feast/cli_utils.py +++ b/sdk/python/feast/cli_utils.py @@ -279,7 +279,7 @@ def handler_list_all_permissions_roles_verbose( for o in objects: permitted_actions = ALL_ACTIONS.copy() for action in ALL_ACTIONS: - # Following code is derived from enforcer.enforce_policy but has a different return type and does not raise PermissionError + # Following code is derived from enforcer.enforce_policy but has a different return type and does not raise FeastPermissionError matching_permissions = [ p for p in permissions diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index ffafe31125..2eed986d7f 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -1,34 +1,52 @@ from typing import Any, List, Set from colorama import Fore, Style +from fastapi import status as HttpStatusCode +from grpc import StatusCode as GrpcStatusCode from feast.field import Field -class DataSourceNotFoundException(Exception): +class FeastError(Exception): + pass + + def rpc_status_code(self) -> GrpcStatusCode: + return GrpcStatusCode.INTERNAL + + def http_status_code(self) -> int: + return HttpStatusCode.HTTP_500_INTERNAL_SERVER_ERROR + + +class DataSourceNotFoundException(FeastError): def __init__(self, path): super().__init__( f"Unable to find table at '{path}'. Please check that table exists." ) -class DataSourceNoNameException(Exception): +class DataSourceNoNameException(FeastError): def __init__(self): super().__init__( "Unable to infer a name for this data source. Either table or name must be specified." ) -class DataSourceRepeatNamesException(Exception): +class DataSourceRepeatNamesException(FeastError): def __init__(self, ds_name: str): super().__init__( f"Multiple data sources share the same case-insensitive name {ds_name}." ) -class FeastObjectNotFoundException(Exception): +class FeastObjectNotFoundException(FeastError): pass + def rpc_status_code(self) -> GrpcStatusCode: + return GrpcStatusCode.NOT_FOUND + + def http_status_code(self) -> int: + return HttpStatusCode.HTTP_404_NOT_FOUND + class EntityNotFoundException(FeastObjectNotFoundException): def __init__(self, name, project=None): @@ -110,49 +128,49 @@ def __init__(self, name: str, project: str): ) -class FeastProviderLoginError(Exception): +class FeastProviderLoginError(FeastError): """Error class that indicates a user has not authenticated with their provider.""" -class FeastProviderNotImplementedError(Exception): +class FeastProviderNotImplementedError(FeastError): def __init__(self, provider_name): super().__init__(f"Provider '{provider_name}' is not implemented") -class FeastRegistryNotSetError(Exception): +class FeastRegistryNotSetError(FeastError): def __init__(self): super().__init__("Registry is not set, but is required") -class FeastFeatureServerTypeInvalidError(Exception): +class FeastFeatureServerTypeInvalidError(FeastError): def __init__(self, feature_server_type: str): super().__init__( f"Feature server type was set to {feature_server_type}, but this type is invalid" ) -class FeastRegistryTypeInvalidError(Exception): +class FeastRegistryTypeInvalidError(FeastError): def __init__(self, registry_type: str): super().__init__( f"Feature server type was set to {registry_type}, but this type is invalid" ) -class FeastModuleImportError(Exception): +class FeastModuleImportError(FeastError): def __init__(self, module_name: str, class_name: str): super().__init__( f"Could not import module '{module_name}' while attempting to load class '{class_name}'" ) -class FeastClassImportError(Exception): +class FeastClassImportError(FeastError): def __init__(self, module_name: str, class_name: str): super().__init__( f"Could not import class '{class_name}' from module '{module_name}'" ) -class FeastExtrasDependencyImportError(Exception): +class FeastExtrasDependencyImportError(FeastError): def __init__(self, extras_type: str, nested_error: str): message = ( nested_error @@ -162,14 +180,14 @@ def __init__(self, extras_type: str, nested_error: str): super().__init__(message) -class FeastOfflineStoreUnsupportedDataSource(Exception): +class FeastOfflineStoreUnsupportedDataSource(FeastError): def __init__(self, offline_store_name: str, data_source_name: str): super().__init__( f"Offline Store '{offline_store_name}' does not support data source '{data_source_name}'" ) -class FeatureNameCollisionError(Exception): +class FeatureNameCollisionError(FeastError): def __init__(self, feature_refs_collisions: List[str], full_feature_names: bool): if full_feature_names: collisions = [ref.replace(":", "__") for ref in feature_refs_collisions] @@ -191,7 +209,7 @@ def __init__(self, feature_refs_collisions: List[str], full_feature_names: bool) ) -class SpecifiedFeaturesNotPresentError(Exception): +class SpecifiedFeaturesNotPresentError(FeastError): def __init__( self, specified_features: List[Field], @@ -204,47 +222,47 @@ def __init__( ) -class SavedDatasetLocationAlreadyExists(Exception): +class SavedDatasetLocationAlreadyExists(FeastError): def __init__(self, location: str): super().__init__(f"Saved dataset location {location} already exists.") -class FeastOfflineStoreInvalidName(Exception): +class FeastOfflineStoreInvalidName(FeastError): def __init__(self, offline_store_class_name: str): super().__init__( f"Offline Store Class '{offline_store_class_name}' should end with the string `OfflineStore`.'" ) -class FeastOnlineStoreInvalidName(Exception): +class FeastOnlineStoreInvalidName(FeastError): def __init__(self, online_store_class_name: str): super().__init__( f"Online Store Class '{online_store_class_name}' should end with the string `OnlineStore`.'" ) -class FeastInvalidAuthConfigClass(Exception): +class FeastInvalidAuthConfigClass(FeastError): def __init__(self, auth_config_class_name: str): super().__init__( f"Auth Config Class '{auth_config_class_name}' should end with the string `AuthConfig`.'" ) -class FeastInvalidBaseClass(Exception): +class FeastInvalidBaseClass(FeastError): def __init__(self, class_name: str, class_type: str): super().__init__( f"Class '{class_name}' should have `{class_type}` as a base class." ) -class FeastOnlineStoreUnsupportedDataSource(Exception): +class FeastOnlineStoreUnsupportedDataSource(FeastError): def __init__(self, online_store_name: str, data_source_name: str): super().__init__( f"Online Store '{online_store_name}' does not support data source '{data_source_name}'" ) -class FeastEntityDFMissingColumnsError(Exception): +class FeastEntityDFMissingColumnsError(FeastError): def __init__(self, expected, missing): super().__init__( f"The entity dataframe you have provided must contain columns {expected}, " @@ -252,7 +270,7 @@ def __init__(self, expected, missing): ) -class FeastJoinKeysDuringMaterialization(Exception): +class FeastJoinKeysDuringMaterialization(FeastError): def __init__( self, source: str, join_key_columns: Set[str], source_columns: Set[str] ): @@ -262,7 +280,7 @@ def __init__( ) -class DockerDaemonNotRunning(Exception): +class DockerDaemonNotRunning(FeastError): def __init__(self): super().__init__( "The Docker Python sdk cannot connect to the Docker daemon. Please make sure you have" @@ -270,7 +288,7 @@ def __init__(self): ) -class RegistryInferenceFailure(Exception): +class RegistryInferenceFailure(FeastError): def __init__(self, repo_obj_type: str, specific_issue: str): super().__init__( f"Inference to fill in missing information for {repo_obj_type} failed. {specific_issue}. " @@ -278,58 +296,58 @@ def __init__(self, repo_obj_type: str, specific_issue: str): ) -class BigQueryJobStillRunning(Exception): +class BigQueryJobStillRunning(FeastError): def __init__(self, job_id): super().__init__(f"The BigQuery job with ID '{job_id}' is still running.") -class BigQueryJobCancelled(Exception): +class BigQueryJobCancelled(FeastError): def __init__(self, job_id): super().__init__(f"The BigQuery job with ID '{job_id}' was cancelled") -class RedshiftCredentialsError(Exception): +class RedshiftCredentialsError(FeastError): def __init__(self): super().__init__("Redshift API failed due to incorrect credentials") -class RedshiftQueryError(Exception): +class RedshiftQueryError(FeastError): def __init__(self, details): super().__init__(f"Redshift SQL Query failed to finish. Details: {details}") -class RedshiftTableNameTooLong(Exception): +class RedshiftTableNameTooLong(FeastError): def __init__(self, table_name: str): super().__init__( f"Redshift table names have a maximum length of 127 characters, but the table name {table_name} has length {len(table_name)} characters." ) -class SnowflakeCredentialsError(Exception): +class SnowflakeCredentialsError(FeastError): def __init__(self): super().__init__("Snowflake Connector failed due to incorrect credentials") -class SnowflakeQueryError(Exception): +class SnowflakeQueryError(FeastError): def __init__(self, details): super().__init__(f"Snowflake SQL Query failed to finish. Details: {details}") -class EntityTimestampInferenceException(Exception): +class EntityTimestampInferenceException(FeastError): def __init__(self, expected_column_name: str): super().__init__( f"Please provide an entity_df with a column named {expected_column_name} representing the time of events." ) -class FeatureViewMissingDuringFeatureServiceInference(Exception): +class FeatureViewMissingDuringFeatureServiceInference(FeastError): def __init__(self, feature_view_name: str, feature_service_name: str): super().__init__( f"Missing {feature_view_name} feature view during inference for {feature_service_name} feature service." ) -class InvalidEntityType(Exception): +class InvalidEntityType(FeastError): def __init__(self, entity_type: type): super().__init__( f"The entity dataframe you have provided must be a Pandas DataFrame or a SQL query, " @@ -337,7 +355,7 @@ def __init__(self, entity_type: type): ) -class ConflictingFeatureViewNames(Exception): +class ConflictingFeatureViewNames(FeastError): # TODO: print file location of conflicting feature views def __init__(self, feature_view_name: str): super().__init__( @@ -345,60 +363,60 @@ def __init__(self, feature_view_name: str): ) -class FeastInvalidInfraObjectType(Exception): +class FeastInvalidInfraObjectType(FeastError): def __init__(self): super().__init__("Could not identify the type of the InfraObject.") -class SnowflakeIncompleteConfig(Exception): +class SnowflakeIncompleteConfig(FeastError): def __init__(self, e: KeyError): super().__init__(f"{e} not defined in a config file or feature_store.yaml file") -class SnowflakeQueryUnknownError(Exception): +class SnowflakeQueryUnknownError(FeastError): def __init__(self, query: str): super().__init__(f"Snowflake query failed: {query}") -class InvalidFeaturesParameterType(Exception): +class InvalidFeaturesParameterType(FeastError): def __init__(self, features: Any): super().__init__( f"Invalid `features` parameter type {type(features)}. Expected one of List[str] and FeatureService." ) -class EntitySQLEmptyResults(Exception): +class EntitySQLEmptyResults(FeastError): def __init__(self, entity_sql: str): super().__init__( f"No entity values found from the specified SQL query to generate the entity dataframe: {entity_sql}." ) -class EntityDFNotDateTime(Exception): +class EntityDFNotDateTime(FeastError): def __init__(self): super().__init__( "The entity dataframe specified does not have the timestamp field as a datetime." ) -class PushSourceNotFoundException(Exception): +class PushSourceNotFoundException(FeastError): def __init__(self, push_source_name: str): super().__init__(f"Unable to find push source '{push_source_name}'.") -class ReadOnlyRegistryException(Exception): +class ReadOnlyRegistryException(FeastError): def __init__(self): super().__init__("Registry implementation is read-only.") -class DataFrameSerializationError(Exception): +class DataFrameSerializationError(FeastError): def __init__(self, input_dict: dict): super().__init__( f"Failed to serialize the provided dictionary into a pandas DataFrame: {input_dict.keys()}" ) -class PermissionNotFoundException(Exception): +class PermissionNotFoundException(FeastError): def __init__(self, name, project): super().__init__(f"Permission {name} does not exist in project {project}") @@ -411,11 +429,22 @@ def __init__(self, name, project=None): super().__init__(f"Permission {name} does not exist") -class ZeroRowsQueryResult(Exception): +class ZeroRowsQueryResult(FeastError): def __init__(self, query: str): super().__init__(f"This query returned zero rows:\n{query}") -class ZeroColumnQueryResult(Exception): +class ZeroColumnQueryResult(FeastError): def __init__(self, query: str): super().__init__(f"This query returned zero columns:\n{query}") + + +class FeastPermissionError(FeastError, PermissionError): + def __init__(self, details: str): + super().__init__(f"Permission error:\n{details}") + + def rpc_status_code(self) -> GrpcStatusCode: + return GrpcStatusCode.PERMISSION_DENIED + + def http_status_code(self) -> int: + return HttpStatusCode.HTTP_403_FORBIDDEN diff --git a/sdk/python/feast/permissions/enforcer.py b/sdk/python/feast/permissions/enforcer.py index ae45b8a78b..d94a81ba04 100644 --- a/sdk/python/feast/permissions/enforcer.py +++ b/sdk/python/feast/permissions/enforcer.py @@ -1,5 +1,6 @@ import logging +from feast.errors import FeastPermissionError from feast.feast_object import FeastObject from feast.permissions.decision import DecisionEvaluator from feast.permissions.permission import ( @@ -29,14 +30,14 @@ def enforce_policy( user: The current user. resources: The resources for which we need to enforce authorized permission. actions: The requested actions to be authorized. - filter_only: If `True`, it removes unauthorized resources from the returned value, otherwise it raises a `PermissionError` the + filter_only: If `True`, it removes unauthorized resources from the returned value, otherwise it raises a `FeastPermissionError` the first unauthorized resource. Defaults to `False`. Returns: list[FeastObject]: A filtered list of the permitted resources. Raises: - PermissionError: If the current user is not authorized to eecute the requested actions on the given resources (and `filter_only` is `False`). + FeastPermissionError: If the current user is not authorized to eecute the requested actions on the given resources (and `filter_only` is `False`). """ if not permissions: return resources @@ -66,12 +67,12 @@ def enforce_policy( if evaluator.is_decided(): grant, explanations = evaluator.grant() if not grant and not filter_only: - raise PermissionError(",".join(explanations)) + raise FeastPermissionError(",".join(explanations)) if grant: _permitted_resources.append(resource) break else: message = f"No permissions defined to manage {actions} on {type(resource)}/{resource.name}." logger.exception(f"**PERMISSION NOT GRANTED**: {message}") - raise PermissionError(message) + raise FeastPermissionError(message) return _permitted_resources diff --git a/sdk/python/feast/permissions/security_manager.py b/sdk/python/feast/permissions/security_manager.py index 2322602388..29c0e06753 100644 --- a/sdk/python/feast/permissions/security_manager.py +++ b/sdk/python/feast/permissions/security_manager.py @@ -67,14 +67,14 @@ def assert_permissions( Args: resources: The resources for which we need to enforce authorized permission. actions: The requested actions to be authorized. - filter_only: If `True`, it removes unauthorized resources from the returned value, otherwise it raises a `PermissionError` the + filter_only: If `True`, it removes unauthorized resources from the returned value, otherwise it raises a `FeastPermissionError` the first unauthorized resource. Defaults to `False`. Returns: list[FeastObject]: A filtered list of the permitted resources, possibly empty. Raises: - PermissionError: If the current user is not authorized to execute all the requested actions on the given resources. + FeastPermissionError: If the current user is not authorized to execute all the requested actions on the given resources. """ return enforce_policy( permissions=self.permissions, @@ -108,7 +108,7 @@ def assert_permissions_to_update( FeastObject: The original `resource`, if permitted. Raises: - PermissionError: If the current user is not authorized to execute all the requested actions on the given resource or on the existing one. + FeastPermissionError: If the current user is not authorized to execute all the requested actions on the given resource or on the existing one. """ actions = [AuthzedAction.DESCRIBE, AuthzedAction.UPDATE] try: @@ -140,7 +140,7 @@ def assert_permissions( FeastObject: The original `resource`, if permitted. Raises: - PermissionError: If the current user is not authorized to execute the requested actions on the given resources. + FeastPermissionError: If the current user is not authorized to execute the requested actions on the given resources. """ sm = get_security_manager() if sm is None: diff --git a/sdk/python/tests/unit/permissions/test_decorator.py b/sdk/python/tests/unit/permissions/test_decorator.py index 8f6c2c420b..92db72c93d 100644 --- a/sdk/python/tests/unit/permissions/test_decorator.py +++ b/sdk/python/tests/unit/permissions/test_decorator.py @@ -1,6 +1,8 @@ import assertpy import pytest +from feast.errors import FeastPermissionError + @pytest.mark.parametrize( "username, can_read, can_write", @@ -22,11 +24,11 @@ def test_access_SecuredFeatureView( if can_read: fv.read_protected() else: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): fv.read_protected() if can_write: fv.write_protected() else: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): fv.write_protected() assertpy.assert_that(fv.unprotected()).is_true() diff --git a/sdk/python/tests/unit/permissions/test_security_manager.py b/sdk/python/tests/unit/permissions/test_security_manager.py index 228dddb01f..d403c8123b 100644 --- a/sdk/python/tests/unit/permissions/test_security_manager.py +++ b/sdk/python/tests/unit/permissions/test_security_manager.py @@ -2,7 +2,7 @@ import pytest from feast.entity import Entity -from feast.errors import FeastObjectNotFoundException +from feast.errors import FeastObjectNotFoundException, FeastPermissionError from feast.permissions.action import READ, AuthzedAction from feast.permissions.security_manager import ( assert_permissions, @@ -66,7 +66,7 @@ def test_access_SecuredFeatureView( result = [] if raise_error_in_permit: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): result = permitted_resources(resources=resources, actions=requested_actions) else: result = permitted_resources(resources=resources, actions=requested_actions) @@ -82,7 +82,7 @@ def test_access_SecuredFeatureView( result = assert_permissions(resource=r, actions=requested_actions) assertpy.assert_that(result).is_equal_to(r) elif raise_error_in_assert[i]: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): assert_permissions(resource=r, actions=requested_actions) else: result = assert_permissions(resource=r, actions=requested_actions) @@ -125,7 +125,7 @@ def getter(name: str, project: str, allow_cache: bool): ) assertpy.assert_that(result).is_equal_to(entity) else: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): assert_permissions_to_update(resource=entity, getter=getter, project="") @@ -165,5 +165,5 @@ def getter(name: str, project: str, allow_cache: bool): ) assertpy.assert_that(result).is_equal_to(entity) else: - with pytest.raises(PermissionError): + with pytest.raises(FeastPermissionError): assert_permissions_to_update(resource=entity, getter=getter, project="")