This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9e80cbcf9 [#4012] improvement(client-python): Refactor Error Handling
in client-python (#4093)
9e80cbcf9 is described below
commit 9e80cbcf9ea8de21eabec9fdb3651d5b376c7b26
Author: noidname01 <[email protected]>
AuthorDate: Tue Jul 9 18:01:33 2024 +0800
[#4012] improvement(client-python): Refactor Error Handling in
client-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]>
---
.../gravitino/catalog/fileset_catalog.py | 18 +++-
.../gravitino/client/gravitino_client.py | 12 ---
.../gravitino/client/gravitino_client_base.py | 2 +-
.../gravitino/client/gravitino_metalake.py | 12 ---
.../gravitino/client/gravitino_version.py | 2 +-
clients/client-python/gravitino/constants/error.py | 71 ++++++++++++
.../gravitino/dto/responses/base_response.py | 5 +-
.../gravitino/dto/responses/error_response.py | 79 ++++++++++++++
.../gravitino/dto/responses/version_response.py | 4 +-
clients/client-python/gravitino/exceptions/base.py | 83 ++++++++++++++
.../exceptions/gravitino_runtime_exception.py | 25 -----
.../__init__.py} | 6 --
.../error_handler.py} | 26 +++--
.../exceptions/handlers/fileset_error_handler.py | 43 ++++++++
.../rest_error_handler.py} | 26 +++--
.../gravitino/exceptions/not_found_exception.py | 24 -----
clients/client-python/gravitino/filesystem/gvfs.py | 2 +-
clients/client-python/gravitino/name_identifier.py | 4 +-
clients/client-python/gravitino/namespace.py | 5 +-
.../client-python/gravitino/rest/rest_message.py | 6 --
.../client-python/gravitino/utils/exceptions.py | 114 --------------------
.../client-python/gravitino/utils/http_client.py | 75 ++++++++++---
clients/client-python/scripts/generate_version.py | 2 +-
.../tests/integration/base_hadoop_env.py | 2 +-
.../tests/integration/hdfs_container.py | 2 +-
.../tests/integration/integration_test_env.py | 2 +-
.../tests/integration/test_fileset_catalog.py | 6 ++
.../tests/integration/test_gvfs_with_hdfs.py | 2 +-
.../tests/unittests/test_error_handler.py | 120 +++++++++++++++++++++
.../tests/unittests/test_gravitino_version.py | 2 +-
.../tests/unittests/test_gvfs_with_local.py | 2 +-
31 files changed, 531 insertions(+), 253 deletions(-)
diff --git a/clients/client-python/gravitino/catalog/fileset_catalog.py
b/clients/client-python/gravitino/catalog/fileset_catalog.py
index 82c345411..5ab2e00e6 100644
--- a/clients/client-python/gravitino/catalog/fileset_catalog.py
+++ b/clients/client-python/gravitino/catalog/fileset_catalog.py
@@ -35,6 +35,7 @@ from gravitino.name_identifier import NameIdentifier
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__)
@@ -88,7 +89,10 @@ class FilesetCatalog(BaseSchemaCatalog):
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()
@@ -114,7 +118,8 @@ class FilesetCatalog(BaseSchemaCatalog):
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()
@@ -163,7 +168,9 @@ class FilesetCatalog(BaseSchemaCatalog):
)
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()
@@ -195,7 +202,9 @@ class FilesetCatalog(BaseSchemaCatalog):
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()
@@ -221,6 +230,7 @@ class FilesetCatalog(BaseSchemaCatalog):
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()
diff --git a/clients/client-python/gravitino/client/gravitino_client.py
b/clients/client-python/gravitino/client/gravitino_client.py
index bd2337a16..21ba63c25 100644
--- a/clients/client-python/gravitino/client/gravitino_client.py
+++ b/clients/client-python/gravitino/client/gravitino_client.py
@@ -26,18 +26,6 @@ from gravitino.client.gravitino_client_base import
GravitinoClientBase
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.
diff --git a/clients/client-python/gravitino/client/gravitino_client_base.py
b/clients/client-python/gravitino/client/gravitino_client_base.py
index 0bf77071a..442b77dee 100644
--- a/clients/client-python/gravitino/client/gravitino_client_base.py
+++ b/clients/client-python/gravitino/client/gravitino_client_base.py
@@ -28,7 +28,7 @@ from gravitino.dto.version_dto import VersionDTO
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
diff --git a/clients/client-python/gravitino/client/gravitino_metalake.py
b/clients/client-python/gravitino/client/gravitino_metalake.py
index e4009fc17..9ef787a07 100644
--- a/clients/client-python/gravitino/client/gravitino_metalake.py
+++ b/clients/client-python/gravitino/client/gravitino_metalake.py
@@ -36,18 +36,6 @@ from gravitino.utils import HTTPClient
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
diff --git a/clients/client-python/gravitino/client/gravitino_version.py
b/clients/client-python/gravitino/client/gravitino_version.py
index f59e78a86..c69376f1b 100644
--- a/clients/client-python/gravitino/client/gravitino_version.py
+++ b/clients/client-python/gravitino/client/gravitino_version.py
@@ -22,7 +22,7 @@ from dataclasses import dataclass
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):
diff --git a/clients/client-python/gravitino/constants/error.py
b/clients/client-python/gravitino/constants/error.py
new file mode 100644
index 000000000..8f4bdbffb
--- /dev/null
+++ b/clients/client-python/gravitino/constants/error.py
@@ -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()}
diff --git a/clients/client-python/gravitino/dto/responses/base_response.py
b/clients/client-python/gravitino/dto/responses/base_response.py
index 71efece86..8ce91bd9b 100644
--- a/clients/client-python/gravitino/dto/responses/base_response.py
+++ b/clients/client-python/gravitino/dto/responses/base_response.py
@@ -35,7 +35,6 @@ class BaseResponse(RESTResponse):
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"
diff --git a/clients/client-python/gravitino/dto/responses/error_response.py
b/clients/client-python/gravitino/dto/responses/error_response.py
new file mode 100644
index 000000000..ad1fd273e
--- /dev/null
+++ b/clients/client-python/gravitino/dto/responses/error_response.py
@@ -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
+ )
diff --git a/clients/client-python/gravitino/dto/responses/version_response.py
b/clients/client-python/gravitino/dto/responses/version_response.py
index 6994dbca1..abeebcabf 100644
--- a/clients/client-python/gravitino/dto/responses/version_response.py
+++ b/clients/client-python/gravitino/dto/responses/version_response.py
@@ -20,8 +20,8 @@ under the License.
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
diff --git a/clients/client-python/gravitino/exceptions/base.py
b/clients/client-python/gravitino/exceptions/base.py
new file mode 100644
index 000000000..96081f1ae
--- /dev/null
+++ b/clients/client-python/gravitino/exceptions/base.py
@@ -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"""
diff --git
a/clients/client-python/gravitino/exceptions/gravitino_runtime_exception.py
b/clients/client-python/gravitino/exceptions/gravitino_runtime_exception.py
deleted file mode 100644
index d98bc169c..000000000
--- a/clients/client-python/gravitino/exceptions/gravitino_runtime_exception.py
+++ /dev/null
@@ -1,25 +0,0 @@
-"""
-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)
diff --git
a/clients/client-python/gravitino/exceptions/no_such_metalake_exception.py
b/clients/client-python/gravitino/exceptions/handlers/__init__.py
similarity index 80%
rename from
clients/client-python/gravitino/exceptions/no_such_metalake_exception.py
rename to clients/client-python/gravitino/exceptions/handlers/__init__.py
index 6dc136ca9..c206137f1 100644
--- a/clients/client-python/gravitino/exceptions/no_such_metalake_exception.py
+++ b/clients/client-python/gravitino/exceptions/handlers/__init__.py
@@ -16,9 +16,3 @@ KIND, either express or implied. See the License for the
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."""
diff --git
a/clients/client-python/gravitino/exceptions/illegal_namespace_exception.py
b/clients/client-python/gravitino/exceptions/handlers/error_handler.py
similarity index 50%
rename from
clients/client-python/gravitino/exceptions/illegal_namespace_exception.py
rename to clients/client-python/gravitino/exceptions/handlers/error_handler.py
index f8ded7070..d1ce2a4fc 100644
--- a/clients/client-python/gravitino/exceptions/illegal_namespace_exception.py
+++ b/clients/client-python/gravitino/exceptions/handlers/error_handler.py
@@ -17,12 +17,24 @@ specific language governing permissions and limitations
under the License.
"""
+from abc import ABC, abstractmethod
+from gravitino.dto.responses.error_response import ErrorResponse
-class IllegalNamespaceException(Exception):
- """An exception thrown when a namespace is invalid."""
- def __init__(self, message=None):
- if message:
- super().__init__(message)
- else:
- super().__init__()
+class ErrorHandler(ABC):
+ """The ErrorHandler class is an abstract class specialized for handling
ErrorResponse objects.
+ Subclasses of ErrorHandler must implement the parseResponse method to
provide custom parsing
+ logic for different types of errors.
+ """
+
+ @abstractmethod
+ def handle(self, error_response: ErrorResponse):
+ """Handles the error response and raise the appropriate Exception. The
implementation will use the
+ provided error response to determine which exception to raise.
+
+ Args:
+ error_response: the error response from the server
+
+ Raises:
+ appropriate Exception.
+ """
diff --git
a/clients/client-python/gravitino/exceptions/handlers/fileset_error_handler.py
b/clients/client-python/gravitino/exceptions/handlers/fileset_error_handler.py
new file mode 100644
index 000000000..bff39d02b
--- /dev/null
+++
b/clients/client-python/gravitino/exceptions/handlers/fileset_error_handler.py
@@ -0,0 +1,43 @@
+"""
+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 gravitino.constants.error import ErrorConstants
+from gravitino.dto.responses.error_response import ErrorResponse
+from gravitino.exceptions.handlers.rest_error_handler import RestErrorHandler
+from gravitino.exceptions.base import NoSuchFilesetException,
NoSuchSchemaException
+
+
+class FilesetErrorHandler(RestErrorHandler):
+
+ def handle(self, error_response: ErrorResponse):
+
+ error_message = error_response.format_error_message()
+ code = error_response.code()
+ exception_type = error_response.type()
+
+ if code == ErrorConstants.NOT_FOUND_CODE:
+ if exception_type == NoSuchSchemaException.__name__:
+ raise NoSuchSchemaException(error_message)
+ if exception_type == NoSuchFilesetException.__name__:
+ raise NoSuchFilesetException(error_message)
+
+ super().handle(error_response)
+
+
+FILESET_ERROR_HANDLER = FilesetErrorHandler()
diff --git
a/clients/client-python/gravitino/exceptions/illegal_name_identifier_exception.py
b/clients/client-python/gravitino/exceptions/handlers/rest_error_handler.py
similarity index 53%
rename from
clients/client-python/gravitino/exceptions/illegal_name_identifier_exception.py
rename to
clients/client-python/gravitino/exceptions/handlers/rest_error_handler.py
index bb14af12c..8319e6254 100644
---
a/clients/client-python/gravitino/exceptions/illegal_name_identifier_exception.py
+++ b/clients/client-python/gravitino/exceptions/handlers/rest_error_handler.py
@@ -17,12 +17,24 @@ specific language governing permissions and limitations
under the License.
"""
+from gravitino.constants.error import ERROR_CODE_MAPPING
+from gravitino.dto.responses.error_response import ErrorResponse
+from gravitino.exceptions.handlers.error_handler import ErrorHandler
+from gravitino.exceptions.base import RESTException
-class IllegalNameIdentifierException(Exception):
- """An exception thrown when a name identifier is invalid."""
- def __init__(self, message=None):
- if message:
- super().__init__(message)
- else:
- super().__init__()
+class RestErrorHandler(ErrorHandler):
+
+ def handle(self, error_response: ErrorResponse):
+ error_message = error_response.format_error_message()
+ code = error_response.code()
+
+ if code in ERROR_CODE_MAPPING:
+ raise ERROR_CODE_MAPPING[code](error_message)
+
+ raise RESTException(
+ f"Unable to process: {error_message}",
+ )
+
+
+REST_ERROR_HANDLER = RestErrorHandler()
diff --git a/clients/client-python/gravitino/exceptions/not_found_exception.py
b/clients/client-python/gravitino/exceptions/not_found_exception.py
deleted file mode 100644
index aff8cb583..000000000
--- a/clients/client-python/gravitino/exceptions/not_found_exception.py
+++ /dev/null
@@ -1,24 +0,0 @@
-"""
-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 gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
-
-
-class NotFoundException(GravitinoRuntimeException):
- """Base class for all exceptions thrown when a resource is not found."""
diff --git a/clients/client-python/gravitino/filesystem/gvfs.py
b/clients/client-python/gravitino/filesystem/gvfs.py
index edb150677..a50c97f4c 100644
--- a/clients/client-python/gravitino/filesystem/gvfs.py
+++ b/clients/client-python/gravitino/filesystem/gvfs.py
@@ -33,7 +33,7 @@ from readerwriterlock import rwlock
from gravitino.api.catalog import Catalog
from gravitino.api.fileset import Fileset
from gravitino.client.gravitino_client import GravitinoClient
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
from gravitino.name_identifier import NameIdentifier
PROTOCOL_NAME = "gvfs"
diff --git a/clients/client-python/gravitino/name_identifier.py
b/clients/client-python/gravitino/name_identifier.py
index 069e22500..8b854b0dc 100644
--- a/clients/client-python/gravitino/name_identifier.py
+++ b/clients/client-python/gravitino/name_identifier.py
@@ -23,13 +23,11 @@ from dataclasses import dataclass, field
from dataclasses_json import DataClassJsonMixin, config
-from gravitino.exceptions.illegal_name_identifier_exception import (
+from gravitino.exceptions.base import (
IllegalNameIdentifierException,
)
from gravitino.namespace import Namespace
-# TODO: delete redundant methods
-
@dataclass
class NameIdentifier(DataClassJsonMixin):
diff --git a/clients/client-python/gravitino/namespace.py
b/clients/client-python/gravitino/namespace.py
index 33747ed25..221fd3ea8 100644
--- a/clients/client-python/gravitino/namespace.py
+++ b/clients/client-python/gravitino/namespace.py
@@ -19,8 +19,7 @@ under the License.
import json
from typing import List, ClassVar
-
-# TODO: delete redundant methods
+from gravitino.exceptions.base import IllegalNamespaceException
class Namespace:
@@ -137,4 +136,4 @@ class Namespace:
args: The arguments to the message.
"""
if not expression:
- raise ValueError(message.format(*args))
+ raise IllegalNamespaceException(message.format(*args))
diff --git a/clients/client-python/gravitino/rest/rest_message.py
b/clients/client-python/gravitino/rest/rest_message.py
index 59e93325b..b85ac6a6c 100644
--- a/clients/client-python/gravitino/rest/rest_message.py
+++ b/clients/client-python/gravitino/rest/rest_message.py
@@ -44,12 +44,6 @@ class RESTMessage(DataClassJsonMixin, ABC):
pass
-class IllegalArgumentException(Exception):
- """Exception raised if a REST message is not valid according to the REST
spec."""
-
- pass
-
-
class RESTRequest(RESTMessage, ABC):
"""Interface to mark a REST request."""
diff --git a/clients/client-python/gravitino/utils/exceptions.py
b/clients/client-python/gravitino/utils/exceptions.py
deleted file mode 100644
index 09314a5e0..000000000
--- a/clients/client-python/gravitino/utils/exceptions.py
+++ /dev/null
@@ -1,114 +0,0 @@
-"""
-MIT License
-
-Copyright (c) 2016 Dhamu
-
-Permission is hereby granted, free of charge, to any person obtaining a copy
-of this software and associated documentation files (the "Software"), to deal
-in the Software without restriction, including without limitation the rights
-to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the Software is
-furnished to do so, subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in all
-copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-SOFTWARE.
-"""
-
-import json
-
-
-class HTTPError(Exception):
- """Base of all other errors"""
-
- def __init__(self, error):
- self.status_code = error.code
- self.reason = error.reason
- self.body = error.read()
- self.headers = error.hdrs
-
- def json(self):
- """
- :Returns: object of response error from the API
- """
- try:
- return json.loads(self.body.decode("utf-8"))
- except json.decoder.JSONDecodeError:
- return {"exception": self.body.decode("utf-8")}
-
- def __str__(self):
- return self.json().get("exception")
-
-
-class BadRequestsError(HTTPError):
- pass
-
-
-class UnauthorizedError(HTTPError):
- pass
-
-
-class ForbiddenError(HTTPError):
- pass
-
-
-class NotFoundError(HTTPError):
- pass
-
-
-class MethodNotAllowedError(HTTPError):
- pass
-
-
-class PayloadTooLargeError(HTTPError):
- pass
-
-
-class UnsupportedMediaTypeError(HTTPError):
- pass
-
-
-class TooManyRequestsError(HTTPError):
- pass
-
-
-class InternalServerError(HTTPError):
- pass
-
-
-class ServiceUnavailableError(HTTPError):
- pass
-
-
-class GatewayTimeoutError(HTTPError):
- pass
-
-
-err_dict = {
- 400: BadRequestsError,
- 401: UnauthorizedError,
- 403: ForbiddenError,
- 404: NotFoundError,
- 405: MethodNotAllowedError,
- 413: PayloadTooLargeError,
- 415: UnsupportedMediaTypeError,
- 429: TooManyRequestsError,
- 500: InternalServerError,
- 503: ServiceUnavailableError,
- 504: GatewayTimeoutError,
-}
-
-
-def handle_error(error):
- try:
- exc = err_dict[error.code](error)
- except KeyError:
- return HTTPError(error)
- return exc
diff --git a/clients/client-python/gravitino/utils/http_client.py
b/clients/client-python/gravitino/utils/http_client.py
index 150c00002..67504f12d 100644
--- a/clients/client-python/gravitino/utils/http_client.py
+++ b/clients/client-python/gravitino/utils/http_client.py
@@ -23,17 +23,23 @@ SOFTWARE.
"""
import logging
+from typing import Tuple
from urllib.request import Request, build_opener
from urllib.parse import urlencode
+
from urllib.error import HTTPError
import json as _json
from gravitino.auth.auth_constants import AuthConstants
from gravitino.auth.auth_data_provider import AuthDataProvider
from gravitino.typing import JSONType
-from gravitino.utils.exceptions import handle_error
+
from gravitino.constants.timeout import TIMEOUT
+from gravitino.dto.responses.error_response import ErrorResponse
+from gravitino.exceptions.base import RESTException, UnknownError
+from gravitino.exceptions.handlers.error_handler import ErrorHandler
+
logger = logging.getLogger(__name__)
@@ -115,16 +121,33 @@ class HTTPClient:
_headers[key] = value
return _headers
- def _make_request(self, opener, request, timeout=None):
+ def _make_request(self, opener, request, timeout=None) -> Tuple[bool,
Response]:
timeout = timeout or self.timeout
try:
- return opener.open(request, timeout=timeout)
+ return (True, Response(opener.open(request, timeout=timeout)))
except HTTPError as err:
- exc = handle_error(err)
- raise exc from None
+ err_body = err.read()
+
+ if err_body is None:
+ return (
+ False,
+ ErrorResponse.generate_error_response(RESTException,
err.reason),
+ )
+
+ err_resp = ErrorResponse.from_json(err_body, infer_missing=True)
+ err_resp.validate()
+
+ return (False, err_resp)
def _request(
- self, method, endpoint, params=None, json=None, headers=None,
timeout=None
+ self,
+ method,
+ endpoint,
+ params=None,
+ json=None,
+ headers=None,
+ timeout=None,
+ error_handler: ErrorHandler = None,
):
method = method.upper()
request_data = None
@@ -154,19 +177,41 @@ class HTTPClient:
self.auth_data_provider.get_token_data().decode("utf-8"),
)
request.get_method = lambda: method
- return Response(self._make_request(opener, request, timeout=timeout))
+ is_success, resp = self._make_request(opener, request, timeout=timeout)
+
+ if is_success:
+ return resp
+
+ if not isinstance(error_handler, ErrorHandler):
+ raise UnknownError(
+ f"Unknown error handler {type(error_handler).__name__}, error
response body: {resp}"
+ ) from None
+
+ error_handler.handle(resp)
+
+ # This code generally will not be run because the error handler should
define the default behavior,
+ # and raise appropriate
+ raise UnknownError(
+ f"Error handler {type(error_handler).__name__} can't handle this
response, error response body: {resp}"
+ ) from None
- def get(self, endpoint, params=None, **kwargs):
- return self._request("get", endpoint, params=params, **kwargs)
+ def get(self, endpoint, params=None, error_handler=None, **kwargs):
+ return self._request(
+ "get", endpoint, params=params, error_handler=error_handler,
**kwargs
+ )
- def delete(self, endpoint, **kwargs):
- return self._request("delete", endpoint, **kwargs)
+ def delete(self, endpoint, error_handler=None, **kwargs):
+ return self._request("delete", endpoint, error_handler=error_handler,
**kwargs)
- def post(self, endpoint, json=None, **kwargs):
- return self._request("post", endpoint, json=json, **kwargs)
+ def post(self, endpoint, json=None, error_handler=None, **kwargs):
+ return self._request(
+ "post", endpoint, json=json, error_handler=error_handler, **kwargs
+ )
- def put(self, endpoint, json=None, **kwargs):
- return self._request("put", endpoint, json=json, **kwargs)
+ def put(self, endpoint, json=None, error_handler=None, **kwargs):
+ return self._request(
+ "put", endpoint, json=json, error_handler=error_handler, **kwargs
+ )
def close(self):
self._request("close", "/")
diff --git a/clients/client-python/scripts/generate_version.py
b/clients/client-python/scripts/generate_version.py
index 262bac5d1..bf7e8c449 100644
--- a/clients/client-python/scripts/generate_version.py
+++ b/clients/client-python/scripts/generate_version.py
@@ -23,7 +23,7 @@ import subprocess
from datetime import datetime
from gravitino.constants.version import Version, VERSION_INI, SETUP_FILE
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
VERSION_PATTERN = r"version\s*=\s*['\"]([^'\"]+)['\"]"
diff --git a/clients/client-python/tests/integration/base_hadoop_env.py
b/clients/client-python/tests/integration/base_hadoop_env.py
index 694331078..9099ed13f 100644
--- a/clients/client-python/tests/integration/base_hadoop_env.py
+++ b/clients/client-python/tests/integration/base_hadoop_env.py
@@ -23,7 +23,7 @@ import shutil
import subprocess
import tarfile
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
logger = logging.getLogger(__name__)
diff --git a/clients/client-python/tests/integration/hdfs_container.py
b/clients/client-python/tests/integration/hdfs_container.py
index 34bc41d86..1c3f1c21d 100644
--- a/clients/client-python/tests/integration/hdfs_container.py
+++ b/clients/client-python/tests/integration/hdfs_container.py
@@ -26,7 +26,7 @@ import docker
from docker import types as tp
from docker.errors import NotFound
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
logger = logging.getLogger(__name__)
diff --git a/clients/client-python/tests/integration/integration_test_env.py
b/clients/client-python/tests/integration/integration_test_env.py
index dde2c2410..8f1a5a2c8 100644
--- a/clients/client-python/tests/integration/integration_test_env.py
+++ b/clients/client-python/tests/integration/integration_test_env.py
@@ -26,7 +26,7 @@ import sys
import requests
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
logger = logging.getLogger(__name__)
diff --git a/clients/client-python/tests/integration/test_fileset_catalog.py
b/clients/client-python/tests/integration/test_fileset_catalog.py
index da79ef1ed..25641ab20 100644
--- a/clients/client-python/tests/integration/test_fileset_catalog.py
+++ b/clients/client-python/tests/integration/test_fileset_catalog.py
@@ -29,6 +29,7 @@ from gravitino import (
Fileset,
FilesetChange,
)
+from gravitino.exceptions.base import NoSuchFilesetException
from tests.integration.integration_test_env import IntegrationTestEnv
logger = logging.getLogger(__name__)
@@ -174,6 +175,11 @@ class TestFilesetCatalog(IntegrationTestEnv):
self.assertEqual(fileset.properties(), self.fileset_properties)
self.assertEqual(fileset.audit_info().creator(), "anonymous")
+ def test_failed_load_fileset(self):
+ catalog = self.gravitino_client.load_catalog(name=self.catalog_name)
+ with self.assertRaises(NoSuchFilesetException):
+ _ =
catalog.as_fileset_catalog().load_fileset(ident=self.fileset_ident)
+
def test_alter_fileset(self):
self.create_fileset()
fileset_propertie_new_value = self.fileset_properties_value2 + "_new"
diff --git a/clients/client-python/tests/integration/test_gvfs_with_hdfs.py
b/clients/client-python/tests/integration/test_gvfs_with_hdfs.py
index 442bbc87e..53fde1223 100644
--- a/clients/client-python/tests/integration/test_gvfs_with_hdfs.py
+++ b/clients/client-python/tests/integration/test_gvfs_with_hdfs.py
@@ -40,7 +40,7 @@ from gravitino import (
Catalog,
Fileset,
)
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
from tests.integration.integration_test_env import IntegrationTestEnv
from tests.integration.hdfs_container import HDFSContainer
from tests.integration.base_hadoop_env import BaseHadoopEnvironment
diff --git a/clients/client-python/tests/unittests/test_error_handler.py
b/clients/client-python/tests/unittests/test_error_handler.py
new file mode 100644
index 000000000..565c08151
--- /dev/null
+++ b/clients/client-python/tests/unittests/test_error_handler.py
@@ -0,0 +1,120 @@
+"""
+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.
+"""
+
+import unittest
+
+from gravitino.dto.responses.error_response import ErrorResponse
+from gravitino.exceptions.base import (
+ NoSuchSchemaException,
+ NoSuchFilesetException,
+ InternalError,
+ RESTException,
+ NotFoundException,
+ IllegalArugmentException,
+ AlreadyExistException,
+ NotEmptyException,
+ UnsupportedOperationException,
+)
+
+from gravitino.exceptions.handlers.rest_error_handler import REST_ERROR_HANDLER
+from gravitino.exceptions.handlers.fileset_error_handler import
FILESET_ERROR_HANDLER
+
+
+class TestErrorHandler(unittest.TestCase):
+
+ def test_rest_error_handler(self):
+
+ with self.assertRaises(RESTException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(RESTException, "mock
error")
+ )
+
+ with self.assertRaises(IllegalArugmentException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ IllegalArugmentException, "mock error"
+ )
+ )
+
+ with self.assertRaises(InternalError):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(InternalError, "mock
error")
+ )
+
+ with self.assertRaises(NotFoundException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(NotFoundException, "mock
error")
+ )
+
+ with self.assertRaises(NotFoundException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ NoSuchFilesetException, "mock error"
+ )
+ )
+
+ with self.assertRaises(AlreadyExistException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ AlreadyExistException, "mock error"
+ )
+ )
+
+ with self.assertRaises(NotEmptyException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(NotEmptyException, "mock
error")
+ )
+
+ with self.assertRaises(UnsupportedOperationException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ UnsupportedOperationException, "mock error"
+ )
+ )
+
+ with self.assertRaises(RESTException):
+ REST_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(Exception, "mock error")
+ )
+
+ def test_fileset_error_handler(self):
+
+ with self.assertRaises(NoSuchFilesetException):
+ FILESET_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ NoSuchFilesetException, "mock error"
+ )
+ )
+
+ with self.assertRaises(NoSuchSchemaException):
+ FILESET_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(
+ NoSuchSchemaException, "mock error"
+ )
+ )
+
+ with self.assertRaises(InternalError):
+ FILESET_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(InternalError, "mock
error")
+ )
+
+ with self.assertRaises(RESTException):
+ FILESET_ERROR_HANDLER.handle(
+ ErrorResponse.generate_error_response(Exception, "mock error")
+ )
diff --git a/clients/client-python/tests/unittests/test_gravitino_version.py
b/clients/client-python/tests/unittests/test_gravitino_version.py
index 02196effe..6f18bcfd1 100644
--- a/clients/client-python/tests/unittests/test_gravitino_version.py
+++ b/clients/client-python/tests/unittests/test_gravitino_version.py
@@ -21,7 +21,7 @@ import unittest
from gravitino.client.gravitino_version import GravitinoVersion
from gravitino.dto.version_dto import VersionDTO
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
class TestGravitinoVersion(unittest.TestCase):
diff --git a/clients/client-python/tests/unittests/test_gvfs_with_local.py
b/clients/client-python/tests/unittests/test_gvfs_with_local.py
index c5cebc764..61ea004a1 100644
--- a/clients/client-python/tests/unittests/test_gvfs_with_local.py
+++ b/clients/client-python/tests/unittests/test_gvfs_with_local.py
@@ -37,7 +37,7 @@ from gravitino import NameIdentifier
from gravitino.dto.audit_dto import AuditDTO
from gravitino.dto.fileset_dto import FilesetDTO
from gravitino.filesystem.gvfs import FilesetContext, StorageType
-from gravitino.exceptions.gravitino_runtime_exception import
GravitinoRuntimeException
+from gravitino.exceptions.base import GravitinoRuntimeException
from tests.unittests import mock_base
