Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 544
Add rename_view to REST Catalog#2149
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
3fe9daa80abad483682cc9acd6d0f9a2e62e88fe55File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -334,6 +334,9 @@ def load_view(self, identifier: str | Identifier) -> View: | ||
| raise NotImplementedError | ||
| @override | ||
| def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None: | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| raise NotImplementedError | ||
| def load_namespace_properties(self, namespace: str | Identifier) -> Properties: | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| dataset_name = self.identifier_to_database(namespace) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -159,6 +159,7 @@ class Endpoints: | ||
| register_view: str = "namespaces/{namespace}/register-view" | ||
| drop_view: str = "namespaces/{namespace}/views/{view}" | ||
| view_exists: str = "namespaces/{namespace}/views/{view}" | ||
| rename_view: str = "views/rename" | ||
| plan_table_scan: str = "namespaces/{namespace}/tables/{table}/plan" | ||
| fetch_scan_tasks: str = "namespaces/{namespace}/tables/{table}/tasks" | ||
| @@ -189,6 +190,7 @@ class Capability: | ||
| V1_VIEW_EXISTS = Endpoint(http_method=HttpMethod.HEAD, path=f"{API_PREFIX}/{Endpoints.view_exists}") | ||
| V1_REGISTER_VIEW = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.register_view}") | ||
| V1_DELETE_VIEW = Endpoint(http_method=HttpMethod.DELETE, path=f"{API_PREFIX}/{Endpoints.drop_view}") | ||
| V1_RENAME_VIEW = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.rename_view}") | ||
| V1_SUBMIT_TABLE_SCAN_PLAN = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.plan_table_scan}") | ||
| V1_TABLE_SCAN_PLAN_TASKS = Endpoint(http_method=HttpMethod.POST, path=f"{API_PREFIX}/{Endpoints.fetch_scan_tasks}") | ||
| @@ -218,6 +220,7 @@ class Capability: | ||
| Capability.V1_LIST_VIEWS, | ||
| Capability.V1_LOAD_VIEW, | ||
| Capability.V1_DELETE_VIEW, | ||
| Capability.V1_RENAME_VIEW, | ||
| ) | ||
| ) | ||
| @@ -1503,6 +1506,29 @@ def drop_view(self, identifier: str | Identifier) -> None: | ||
| except HTTPError as exc: | ||
| _handle_non_200_response(exc, {404: NoSuchViewError}) | ||
| @retry(**_RETRY_ARGS) | ||
| def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None: | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| self._check_endpoint(Capability.V1_RENAME_VIEW) | ||
| payload = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. self._split_identifier_for_json is still called twice per namespace. | ||
| "source": self._split_identifier_for_json(from_identifier), | ||
| "destination": self._split_identifier_for_json(to_identifier), | ||
| } | ||
| # Ensure source and destination namespaces exist before rename. | ||
| source_namespace = self._split_identifier_for_json(from_identifier)["namespace"] | ||
| dest_namespace = self._split_identifier_for_json(to_identifier)["namespace"] | ||
| if not self.namespace_exists(source_namespace): | ||
| raise NoSuchNamespaceError(f"Source namespace does not exist: {source_namespace}") | ||
| if not self.namespace_exists(dest_namespace): | ||
| raise NoSuchNamespaceError(f"Destination namespace does not exist: {dest_namespace}") | ||
| response = self._session.post(self.url(Endpoints.rename_view), json=payload) | ||
| try: | ||
| response.raise_for_status() | ||
| except HTTPError as exc: | ||
| _handle_non_200_response(exc, {404: NoSuchViewError, 409: ViewAlreadyExistsError}) | ||
| def close(self) -> None: | ||
| """Close the catalog and release Session connection adapters. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -784,3 +784,6 @@ def close(self) -> None: | ||
| """ | ||
| if hasattr(self, "engine"): | ||
| self.engine.dispose() | ||
| def rename_view(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> None: | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| raise NotImplementedError | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -110,6 +110,7 @@ | ||
| Capability.V1_VIEW_EXISTS, | ||
| Capability.V1_REGISTER_VIEW, | ||
| Capability.V1_DELETE_VIEW, | ||
| Capability.V1_RENAME_VIEW, | ||
| Capability.V1_SUBMIT_TABLE_SCAN_PLAN, | ||
| Capability.V1_TABLE_SCAN_PLAN_TASKS, | ||
| ] | ||
| @@ -3196,3 +3197,122 @@ def test_load_table_without_storage_credentials( | ||
| ) | ||
| assert actual.metadata.model_dump() == expected.metadata.model_dump() | ||
| assert actual == expected | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| def test_rename_view_204(rest_mock: Mocker) -> None: | ||
| from_identifier = ("some_namespace", "old_view") | ||
| to_identifier = ("some_namespace", "new_view") | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/some_namespace", | ||
| status_code=200, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| rest_mock.post( | ||
| f"{TEST_URI}v1/views/rename", | ||
| json={ | ||
| "source": {"namespace": ["some_namespace"], "name": "old_view"}, | ||
| "destination": {"namespace": ["some_namespace"], "name": "new_view"}, | ||
| }, | ||
| status_code=204, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) | ||
| catalog.rename_view(from_identifier, to_identifier) | ||
| assert ( | ||
| rest_mock.last_request.text == """{"source": {"namespace": ["some_namespace"], "name": "old_view"}, """ | ||
| """"destination": {"namespace": ["some_namespace"], "name": "new_view"}}""" | ||
| ) | ||
| def test_rename_view_404(rest_mock: Mocker) -> None: | ||
| from_identifier = ("some_namespace", "non_existent_view") | ||
| to_identifier = ("some_namespace", "new_view") | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/some_namespace", | ||
| status_code=200, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| rest_mock.post( | ||
| f"{TEST_URI}v1/views/rename", | ||
| json={ | ||
| "error": { | ||
| "message": "View does not exist: some_namespace.non_existent_view", | ||
| "type": "NoSuchViewException", | ||
| "code": 404, | ||
| } | ||
| }, | ||
| status_code=404, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) | ||
| with pytest.raises(NoSuchViewError) as exc_info: | ||
| catalog.rename_view(from_identifier, to_identifier) | ||
| assert "View does not exist: some_namespace.non_existent_view" in str(exc_info.value) | ||
| def test_rename_view_409(rest_mock: Mocker) -> None: | ||
| from_identifier = ("some_namespace", "old_view") | ||
| to_identifier = ("some_namespace", "existing_view") | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/some_namespace", | ||
| status_code=200, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| rest_mock.post( | ||
| f"{TEST_URI}v1/views/rename", | ||
| json={ | ||
| "error": { | ||
| "message": "View already exists: some_namespace.existing_view", | ||
| "type": "ViewAlreadyExistsException", | ||
| "code": 409, | ||
| } | ||
| }, | ||
| status_code=409, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) | ||
| with pytest.raises(ViewAlreadyExistsError) as exc_info: | ||
| catalog.rename_view(from_identifier, to_identifier) | ||
| assert "View already exists: some_namespace.existing_view" in str(exc_info.value) | ||
| def test_rename_view_source_namespace_does_not_exist(rest_mock: Mocker) -> None: | ||
| from_identifier = ("non_existent_namespace", "old_view") | ||
| to_identifier = ("some_namespace", "new_view") | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/non_existent_namespace", | ||
| status_code=404, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/some_namespace", | ||
| status_code=200, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) | ||
| with pytest.raises(NoSuchNamespaceError) as exc_info: | ||
| catalog.rename_view(from_identifier, to_identifier) | ||
| assert "Source namespace does not exist: ('non_existent_namespace',)" in str(exc_info.value) | ||
| def test_rename_view_destination_namespace_does_not_exist(rest_mock: Mocker) -> None: | ||
| from_identifier = ("some_namespace", "old_view") | ||
| to_identifier = ("non_existent_namespace", "new_view") | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/some_namespace", | ||
| status_code=200, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| rest_mock.head( | ||
| f"{TEST_URI}v1/namespaces/non_existent_namespace", | ||
| status_code=404, | ||
| request_headers=TEST_HEADERS, | ||
| ) | ||
| catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) | ||
| with pytest.raises(NoSuchNamespaceError) as exc_info: | ||
| catalog.rename_view(from_identifier, to_identifier) | ||
| assert "Destination namespace does not exist: ('non_existent_namespace',)" in str(exc_info.value) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -670,6 +670,26 @@ def test_rest_drop_view( | ||
| @pytest.mark.integration | ||
| def test_rest_rename_view( | ||
rambleraptor marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| rest_catalog: RestCatalog, example_view_metadata_v1: dict[str, Any], database_name: str, view_name: str | ||
| ) -> None: | ||
| from_identifier = (database_name, view_name) | ||
| to_identifier = (database_name, f"{view_name}_renamed") | ||
| rest_catalog.create_namespace_if_not_exists(database_name) | ||
| view = View(from_identifier, ViewMetadata.model_validate(example_view_metadata_v1)) | ||
| rest_catalog.create_view(from_identifier, view.schema(), view.current_version()) | ||
| assert rest_catalog.view_exists(from_identifier) | ||
| rest_catalog.rename_view(from_identifier, to_identifier) | ||
| assert not rest_catalog.view_exists(from_identifier) | ||
| assert rest_catalog.view_exists(to_identifier) | ||
| @pytest.mark.integration | ||
| @pytest.mark.skip(reason="Requires Iceberg REST Fixtures 1.11.x") | ||
| def test_rest_custom_namespace_separator(rest_catalog: RestCatalog, table_schema_simple: Schema) -> None: | ||
| """ | ||
| Tests that the REST catalog correctly picks up the namespace-separator from the config endpoint. | ||
Uh oh!
There was an error while loading. Please reload this page.