Skip to content

Commit

Permalink
[#4012] improvement(client-python): Refactor Error Handling in client…
Browse files Browse the repository at this point in the history
…-python (#4093)

### What changes were proposed in this pull request?

* Refactor the error handling structure, each API can implement their
own error handler to raise custom exceptions
* Add unit test for error handler, but unit tests and integration tests
for each API(ex. metalake, catalog, schema) have not been added, I will
create issues for them.
	- [ ] Add Metalake Error Handler and related exceptions, test cases
	- [ ] Add Catalog Error Handler and related exceptions, test cases
	- [ ] Add Schema Error Handler and related exceptions, test cases
	- [ ] Add OAuth Error Handler and related exceptions, test cases
* Create `gravitino/exceptions/base.py` to define all the exceptions.
* Remove some unused files and exceptions

### Why are the changes needed?

Fix: #4012

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

UT added and one IT added, test with `./gradlew
clients:client-python:test`

---------

Co-authored-by: TimWang <[email protected]>
  • Loading branch information
noidname01 and TimWang authored Jul 9, 2024
1 parent 7c8fc04 commit 9e80cbc
Show file tree
Hide file tree
Showing 31 changed files with 531 additions and 253 deletions.
18 changes: 14 additions & 4 deletions clients/client-python/gravitino/catalog/fileset_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from gravitino.namespace import Namespace
from gravitino.utils import HTTPClient
from gravitino.rest.rest_utils import encode_string
from gravitino.exceptions.handlers.fileset_error_handler import FILESET_ERROR_HANDLER

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -88,7 +89,10 @@ def list_filesets(self, namespace: Namespace) -> List[NameIdentifier]:

full_namespace = self._get_fileset_full_namespace(namespace)

resp = self.rest_client.get(self.format_fileset_request_path(full_namespace))
resp = self.rest_client.get(
self.format_fileset_request_path(full_namespace),
error_handler=FILESET_ERROR_HANDLER,
)
entity_list_resp = EntityListResponse.from_json(resp.body, infer_missing=True)
entity_list_resp.validate()

Expand All @@ -114,7 +118,8 @@ def load_fileset(self, ident: NameIdentifier) -> Fileset:
full_namespace = self._get_fileset_full_namespace(ident.namespace())

resp = self.rest_client.get(
f"{self.format_fileset_request_path(full_namespace)}/{encode_string(ident.name())}"
f"{self.format_fileset_request_path(full_namespace)}/{encode_string(ident.name())}",
error_handler=FILESET_ERROR_HANDLER,
)
fileset_resp = FilesetResponse.from_json(resp.body, infer_missing=True)
fileset_resp.validate()
Expand Down Expand Up @@ -163,7 +168,9 @@ def create_fileset(
)

resp = self.rest_client.post(
self.format_fileset_request_path(full_namespace), req
self.format_fileset_request_path(full_namespace),
req,
error_handler=FILESET_ERROR_HANDLER,
)
fileset_resp = FilesetResponse.from_json(resp.body, infer_missing=True)
fileset_resp.validate()
Expand Down Expand Up @@ -195,7 +202,9 @@ def alter_fileset(self, ident: NameIdentifier, *changes) -> Fileset:
req.validate()

resp = self.rest_client.put(
f"{self.format_fileset_request_path(full_namespace)}/{ident.name()}", req
f"{self.format_fileset_request_path(full_namespace)}/{ident.name()}",
req,
error_handler=FILESET_ERROR_HANDLER,
)
fileset_resp = FilesetResponse.from_json(resp.body, infer_missing=True)
fileset_resp.validate()
Expand All @@ -221,6 +230,7 @@ def drop_fileset(self, ident: NameIdentifier) -> bool:

resp = self.rest_client.delete(
f"{self.format_fileset_request_path(full_namespace)}/{ident.name()}",
error_handler=FILESET_ERROR_HANDLER,
)
drop_resp = DropResponse.from_json(resp.body, infer_missing=True)
drop_resp.validate()
Expand Down
12 changes: 0 additions & 12 deletions clients/client-python/gravitino/client/gravitino_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@
from gravitino.client.gravitino_metalake import GravitinoMetalake


class NoSuchMetalakeException(Exception):
pass


class NoSuchCatalogException(Exception):
pass


class CatalogAlreadyExistsException(Exception):
pass


class GravitinoClient(GravitinoClientBase):
"""Gravitino Client for a user to interact with the Gravitino API, allowing the client to list,
load, create, and alter Catalog.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from gravitino.dto.responses.metalake_response import MetalakeResponse
from gravitino.dto.responses.version_response import VersionResponse
from gravitino.utils import HTTPClient
from gravitino.exceptions.gravitino_runtime_exception import GravitinoRuntimeException
from gravitino.exceptions.base import GravitinoRuntimeException
from gravitino.constants.version import VERSION_INI, Version
from gravitino.name_identifier import NameIdentifier

Expand Down
12 changes: 0 additions & 12 deletions clients/client-python/gravitino/client/gravitino_metalake.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,6 @@
logger = logging.getLogger(__name__)


class NoSuchMetalakeException(Exception):
pass


class NoSuchCatalogException(Exception):
pass


class CatalogAlreadyExistsException(Exception):
pass


class GravitinoMetalake(MetalakeDTO):
"""
Gravitino Metalake is the top-level metadata repository for users. It contains a list of catalogs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from enum import Enum

from gravitino.dto.version_dto import VersionDTO
from gravitino.exceptions.gravitino_runtime_exception import GravitinoRuntimeException
from gravitino.exceptions.base import GravitinoRuntimeException


class Version(Enum):
Expand Down
71 changes: 71 additions & 0 deletions clients/client-python/gravitino/constants/error.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
"""

from enum import IntEnum

from gravitino.exceptions.base import (
RESTException,
IllegalArugmentException,
NotFoundException,
InternalError,
AlreadyExistException,
NotEmptyException,
UnsupportedOperationException,
)


class ErrorConstants(IntEnum):
"""Constants representing error codes for responses."""

# Error codes for REST responses error.
REST_ERROR_CODE = 1000

# Error codes for illegal arguments.
ILLEGAL_ARGUMENTS_CODE = 1001

# Error codes for internal errors.
INTERNAL_ERROR_CODE = 1002

# Error codes for not found.
NOT_FOUND_CODE = 1003

# Error codes for already exists.
ALREADY_EXISTS_CODE = 1004

# Error codes for non empty.
NON_EMPTY_CODE = 1005

# Error codes for unsupported operation.
UNSUPPORTED_OPERATION_CODE = 1006

# Error codes for invalid state.
UNKNOWN_ERROR_CODE = 1100


EXCEPTION_MAPPING = {
RESTException: ErrorConstants.REST_ERROR_CODE,
IllegalArugmentException: ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
InternalError: ErrorConstants.INTERNAL_ERROR_CODE,
NotFoundException: ErrorConstants.NOT_FOUND_CODE,
AlreadyExistException: ErrorConstants.ALREADY_EXISTS_CODE,
NotEmptyException: ErrorConstants.NON_EMPTY_CODE,
UnsupportedOperationException: ErrorConstants.UNSUPPORTED_OPERATION_CODE,
}

ERROR_CODE_MAPPING = {v: k for k, v in EXCEPTION_MAPPING.items()}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def code(self) -> int:

def validate(self):
"""Validates the response code.
TODO: @throws IllegalArgumentException if code value is negative.
@throws IllegalArgumentException if code value is negative.
"""
if self._code < 0:
raise ValueError("code must be >= 0")
assert self._code >= 0, "code must be >= 0"
79 changes: 79 additions & 0 deletions clients/client-python/gravitino/dto/responses/error_response.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
"""
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
"""

from typing import List, Optional
from dataclasses import dataclass, field
from dataclasses_json import config

from gravitino.dto.responses.base_response import BaseResponse
from gravitino.constants.error import ErrorConstants, EXCEPTION_MAPPING
from gravitino.exceptions.base import UnknownError


@dataclass
class ErrorResponse(BaseResponse):
"""Represents an error response."""

_type: str = field(metadata=config(field_name="type"))
_message: str = field(metadata=config(field_name="message"))
_stack: Optional[List[str]] = field(metadata=config(field_name="stack"))

def type(self):
return self._type

def message(self):
return self._message

def stack(self):
return self._stack

def validate(self):
super().validate()

assert (
self._type is not None and len(self._type) != 0
), "type cannot be None or empty"
assert (
self._message is not None and len(self._message) != 0
), "message cannot be None or empty"

def __repr__(self) -> str:
return (
f"ErrorResponse(code={self._code}, type={self._type}, message={self._message})"
+ ("\n\t" + "\n\t".join(self._stack) if self._stack is not None else "")
)

def format_error_message(self) -> str:
return (
f"{self._message}\n" + "\n".join(self._stack)
if self._stack is not None
else ""
)

@classmethod
def generate_error_response(
cls, exception: Exception, message: str
) -> "ErrorResponse":
for exception_type, error_code in EXCEPTION_MAPPING.items():
if issubclass(exception, exception_type):
return cls(error_code, exception.__name__, message, None)

return cls(
ErrorConstants.UNKNOWN_ERROR_CODE, UnknownError.__name__, message, None
)
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from dataclasses import dataclass, field
from dataclasses_json import config

from .base_response import BaseResponse
from ..version_dto import VersionDTO
from gravitino.dto.responses.base_response import BaseResponse
from gravitino.dto.version_dto import VersionDTO


@dataclass
Expand Down
83 changes: 83 additions & 0 deletions clients/client-python/gravitino/exceptions/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
"""
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
"""


class GravitinoRuntimeException(RuntimeError):
"""Base class for all Gravitino runtime exceptions."""

def __init__(self, message, *args):
super().__init__(message % args)


class RESTException(RuntimeError):
"""An exception thrown when a REST request fails."""

def __init__(self, message, *args):
super().__init__(message % args)


class IllegalArugmentException(ValueError):
"""Base class for all exceptions thrown when arguments are invalid."""

def __init__(self, message, *args):
super().__init__(message % args)


class IllegalNameIdentifierException(IllegalArugmentException):
"""An exception thrown when a name identifier is invalid."""


class IllegalNamespaceException(IllegalArugmentException):
"""An exception thrown when a namespace is invalid."""


class InternalError(GravitinoRuntimeException):
"""Base class for all exceptions thrown internally"""


class NotFoundException(GravitinoRuntimeException):
"""Base class for all exceptions thrown when a resource is not found."""


class NoSuchSchemaException(NotFoundException):
"""An exception thrown when a schema is not found."""


class NoSuchFilesetException(NotFoundException):
"""Exception thrown when a file with specified name is not existed."""


class NoSuchMetalakeException(NotFoundException):
"""An exception thrown when a metalake is not found."""


class AlreadyExistException(GravitinoRuntimeException):
"""Base exception thrown when an entity or resource already exists."""


class NotEmptyException(GravitinoRuntimeException):
"""Base class for all exceptions thrown when a resource is not empty."""


class UnsupportedOperationException(GravitinoRuntimeException):
"""Base class for all exceptions thrown when an operation is unsupported"""


class UnknownError(RuntimeError):
"""An exception thrown when other unknown exception is thrown"""
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,3 @@
specific language governing permissions and limitations
under the License.
"""

from gravitino.exceptions.not_found_exception import NotFoundException


class NoSuchMetalakeException(NotFoundException):
"""An exception thrown when a metalake is not found."""
Loading

0 comments on commit 9e80cbc

Please sign in to comment.