Skip to content

Add drop_view to the rest catalog #820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions pyiceberg/catalog/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,17 @@ def update_namespace_properties(
ValueError: If removals and updates have overlapping keys.
"""

@abstractmethod
def drop_view(self, identifier: Union[str, Identifier]) -> None:
"""Drop a view.

Args:
identifier (str | Identifier): View identifier.

Raises:
NoSuchViewError: If a view with the given name does not exist.
"""

@deprecated(
deprecated_in="0.8.0",
removed_in="0.9.0",
Expand Down
3 changes: 3 additions & 0 deletions pyiceberg/catalog/dynamodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,9 @@ def update_namespace_properties(
def list_views(self, namespace: Union[str, Identifier]) -> List[Identifier]:
raise NotImplementedError

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError

def _get_iceberg_table_item(self, database_name: str, table_name: str) -> Dict[str, Any]:
try:
return self._get_dynamo_item(identifier=f"{database_name}.{table_name}", namespace=database_name)
Expand Down
3 changes: 3 additions & 0 deletions pyiceberg/catalog/glue.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,3 +772,6 @@ def update_namespace_properties(

def list_views(self, namespace: Union[str, Identifier]) -> List[Identifier]:
raise NotImplementedError

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError
3 changes: 3 additions & 0 deletions pyiceberg/catalog/hive.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,3 +707,6 @@ def update_namespace_properties(
expected_to_change = (removals or set()).difference(removed)

return PropertiesUpdateSummary(removed=list(removed or []), updated=list(updated or []), missing=list(expected_to_change))

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError
3 changes: 3 additions & 0 deletions pyiceberg/catalog/noop.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,6 @@ def update_namespace_properties(

def list_views(self, namespace: Union[str, Identifier]) -> List[Identifier]:
raise NotImplementedError

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError
35 changes: 30 additions & 5 deletions pyiceberg/catalog/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from enum import Enum
from json import JSONDecodeError
from typing import (
TYPE_CHECKING,
Expand Down Expand Up @@ -48,8 +49,10 @@
ForbiddenError,
NamespaceAlreadyExistsError,
NamespaceNotEmptyError,
NoSuchIdentifierError,
NoSuchNamespaceError,
NoSuchTableError,
NoSuchViewError,
OAuthError,
RESTError,
ServerError,
Expand Down Expand Up @@ -97,6 +100,12 @@ class Endpoints:
get_token: str = "oauth/tokens"
rename_table: str = "tables/rename"
list_views: str = "namespaces/{namespace}/views"
drop_view: str = "namespaces/{namespace}/views/{view}"


class IdentifierKind(Enum):
TABLE = "table"
VIEW = "view"


AUTHORIZATION_HEADER = "Authorization"
Expand Down Expand Up @@ -389,17 +398,20 @@ def _fetch_config(self) -> None:
def _identifier_to_validated_tuple(self, identifier: Union[str, Identifier]) -> Identifier:
identifier_tuple = self.identifier_to_tuple(identifier)
if len(identifier_tuple) <= 1:
raise NoSuchTableError(f"Missing namespace or invalid identifier: {'.'.join(identifier_tuple)}")
raise NoSuchIdentifierError(f"Missing namespace or invalid identifier: {'.'.join(identifier_tuple)}")
return identifier_tuple

def _split_identifier_for_path(self, identifier: Union[str, Identifier, TableIdentifier]) -> Properties:
def _split_identifier_for_path(
self, identifier: Union[str, Identifier, TableIdentifier], kind: IdentifierKind = IdentifierKind.TABLE
) -> Properties:
if isinstance(identifier, TableIdentifier):
if identifier.namespace.root[0] == self.name:
return {"namespace": NAMESPACE_SEPARATOR.join(identifier.namespace.root[1:]), "table": identifier.name}
return {"namespace": NAMESPACE_SEPARATOR.join(identifier.namespace.root[1:]), kind.value: identifier.name}
else:
return {"namespace": NAMESPACE_SEPARATOR.join(identifier.namespace.root), "table": identifier.name}
return {"namespace": NAMESPACE_SEPARATOR.join(identifier.namespace.root), kind.value: identifier.name}
identifier_tuple = self._identifier_to_validated_tuple(identifier)
return {"namespace": NAMESPACE_SEPARATOR.join(identifier_tuple[:-1]), "table": identifier_tuple[-1]}

return {"namespace": NAMESPACE_SEPARATOR.join(identifier_tuple[:-1]), kind.value: identifier_tuple[-1]}

def _split_identifier_for_json(self, identifier: Union[str, Identifier]) -> Dict[str, Union[Identifier, str]]:
identifier_tuple = self._identifier_to_validated_tuple(identifier)
Expand Down Expand Up @@ -867,3 +879,16 @@ def table_exists(self, identifier: Union[str, Identifier]) -> bool:
self._handle_non_200_response(exc, {})

return False

@retry(**_RETRY_ARGS)
def drop_view(self, identifier: Union[str]) -> None:
identifier_tuple = self.identifier_to_tuple_without_catalog(identifier)
response = self._session.delete(
self.url(
Endpoints.drop_view, prefixed=True, **self._split_identifier_for_path(identifier_tuple, IdentifierKind.VIEW)
),
)
try:
response.raise_for_status()
except HTTPError as exc:
self._handle_non_200_response(exc, {404: NoSuchViewError})
3 changes: 3 additions & 0 deletions pyiceberg/catalog/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,6 @@ def update_namespace_properties(

def list_views(self, namespace: Union[str, Identifier]) -> List[Identifier]:
raise NotImplementedError

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError
8 changes: 8 additions & 0 deletions pyiceberg/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ class NoSuchIcebergTableError(NoSuchTableError):
"""Raises when the table found in the REST catalog is not an iceberg table."""


class NoSuchViewError(Exception):
"""Raises when the view can't be found in the REST catalog."""


class NoSuchIdentifierError(Exception):
"""Raises when the identifier can't be found in the REST catalog."""


class NoSuchNamespaceError(Exception):
"""Raised when a referenced name-space is not found."""

Expand Down
3 changes: 3 additions & 0 deletions tests/catalog/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ def update_namespace_properties(
def list_views(self, namespace: Optional[Union[str, Identifier]] = None) -> List[Identifier]:
raise NotImplementedError

def drop_view(self, identifier: Union[str, Identifier]) -> None:
raise NotImplementedError


@pytest.fixture
def catalog(tmp_path: PosixPath) -> InMemoryCatalog:
Expand Down
48 changes: 44 additions & 4 deletions tests/catalog/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
AuthorizationExpiredError,
NamespaceAlreadyExistsError,
NamespaceNotEmptyError,
NoSuchIdentifierError,
NoSuchNamespaceError,
NoSuchTableError,
NoSuchViewError,
OAuthError,
ServerError,
TableAlreadyExistsError,
Expand Down Expand Up @@ -1158,31 +1160,31 @@ def test_delete_table_404(rest_mock: Mocker) -> None:

def test_create_table_missing_namespace(rest_mock: Mocker, table_schema_simple: Schema) -> None:
table = "table"
with pytest.raises(NoSuchTableError) as e:
with pytest.raises(NoSuchIdentifierError) as e:
# Missing namespace
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).create_table(table, table_schema_simple)
assert f"Missing namespace or invalid identifier: {table}" in str(e.value)


def test_load_table_invalid_namespace(rest_mock: Mocker) -> None:
table = "table"
with pytest.raises(NoSuchTableError) as e:
with pytest.raises(NoSuchIdentifierError) as e:
# Missing namespace
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).load_table(table)
assert f"Missing namespace or invalid identifier: {table}" in str(e.value)


def test_drop_table_invalid_namespace(rest_mock: Mocker) -> None:
table = "table"
with pytest.raises(NoSuchTableError) as e:
with pytest.raises(NoSuchIdentifierError) as e:
# Missing namespace
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).drop_table(table)
assert f"Missing namespace or invalid identifier: {table}" in str(e.value)


def test_purge_table_invalid_namespace(rest_mock: Mocker) -> None:
table = "table"
with pytest.raises(NoSuchTableError) as e:
with pytest.raises(NoSuchIdentifierError) as e:
# Missing namespace
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).purge_table(table)
assert f"Missing namespace or invalid identifier: {table}" in str(e.value)
Expand Down Expand Up @@ -1307,3 +1309,41 @@ def test_table_identifier_in_commit_table_request(rest_mock: Mocker, example_tab
rest_mock.last_request.text
== """{"identifier":{"namespace":["namespace"],"name":"table_name"},"requirements":[],"updates":[]}"""
)


def test_drop_view_invalid_namespace(rest_mock: Mocker) -> None:
view = "view"
with pytest.raises(NoSuchIdentifierError) as e:
# Missing namespace
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).drop_view(view)

assert f"Missing namespace or invalid identifier: {view}" in str(e.value)


def test_drop_view_404(rest_mock: Mocker) -> None:
rest_mock.delete(
f"{TEST_URI}v1/namespaces/some_namespace/views/does_not_exists",
json={
"error": {
"message": "The given view does not exist",
"type": "NoSuchViewException",
"code": 404,
}
},
status_code=404,
request_headers=TEST_HEADERS,
)

with pytest.raises(NoSuchViewError) as e:
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).drop_view(("some_namespace", "does_not_exists"))
assert "The given view does not exist" in str(e.value)


def test_drop_view_204(rest_mock: Mocker) -> None:
rest_mock.delete(
f"{TEST_URI}v1/namespaces/some_namespace/views/some_view",
json={},
status_code=204,
request_headers=TEST_HEADERS,
)
RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).drop_view(("some_namespace", "some_view"))