Bug Fix: metadata_location to be optional in TableResponse (#1321)

* make metadata_location optional * add test for staged creation * revert * revert from testing
apache · Nov 14, 2024 · 3ccdc44 · 3ccdc44
1 parent 2ba86b5
commit 3ccdc44
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 2 deletions.
diff --git a/pyiceberg/catalog/rest.py b/pyiceberg/catalog/rest.py
@@ -156,7 +156,7 @@ def _retry_hook(retry_state: RetryCallState) -> None:
 
 
 class TableResponse(IcebergBaseModel):
-    metadata_location: Optional[str] = Field(alias="metadata-location")
+    metadata_location: Optional[str] = Field(alias="metadata-location", default=None)
     metadata: TableMetadata
     config: Properties = Field(default_factory=dict)
 
@@ -599,7 +599,6 @@ def _create_table(
             response.raise_for_status()
         except HTTPError as exc:
             self._handle_non_200_response(exc, {409: TableAlreadyExistsError})
-
         return TableResponse(**response.json())
 
     @retry(**_RETRY_ARGS)

diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py
@@ -79,6 +79,17 @@ def example_table_metadata_with_snapshot_v1_rest_json(example_table_metadata_wit
     }
 
 
+@pytest.fixture
+def example_table_metadata_with_no_location(example_table_metadata_with_snapshot_v1: Dict[str, Any]) -> Dict[str, Any]:
+    return {
+        "metadata": example_table_metadata_with_snapshot_v1,
+        "config": {
+            "client.factory": "io.tabular.iceberg.catalog.TabularAwsClientFactory",
+            "region": "us-west-2",
+        },
+    }
+
+
 @pytest.fixture
 def example_table_metadata_no_snapshot_v1_rest_json(example_table_metadata_no_snapshot_v1: Dict[str, Any]) -> Dict[str, Any]:
     return {
@@ -899,6 +910,70 @@ def test_create_table_with_given_location_removes_trailing_slash_200(
     assert rest_mock.last_request.json()["location"] == location
 
 
+def test_create_staged_table_200(
+    rest_mock: Mocker,
+    table_schema_simple: Schema,
+    example_table_metadata_with_no_location: Dict[str, Any],
+    example_table_metadata_no_snapshot_v1_rest_json: Dict[str, Any],
+) -> None:
+    rest_mock.post(
+        f"{TEST_URI}v1/namespaces/fokko/tables",
+        json=example_table_metadata_with_no_location,
+        status_code=200,
+        request_headers=TEST_HEADERS,
+    )
+    rest_mock.post(
+        f"{TEST_URI}v1/namespaces/fokko/tables/fokko2",
+        json=example_table_metadata_no_snapshot_v1_rest_json,
+        status_code=200,
+        request_headers=TEST_HEADERS,
+    )
+    identifier = ("fokko", "fokko2")
+    catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
+    txn = catalog.create_table_transaction(
+        identifier=identifier,
+        schema=table_schema_simple,
+        location=None,
+        partition_spec=PartitionSpec(
+            PartitionField(source_id=1, field_id=1000, transform=TruncateTransform(width=3), name="id"), spec_id=1
+        ),
+        sort_order=SortOrder(SortField(source_id=2, transform=IdentityTransform())),
+        properties={"owner": "fokko"},
+    )
+    txn.commit_transaction()
+
+    actual_response = rest_mock.last_request.json()
+    expected = {
+        "identifier": {"namespace": ["fokko"], "name": "fokko2"},
+        "requirements": [{"type": "assert-create"}],
+        "updates": [
+            {"action": "assign-uuid", "uuid": "b55d9dda-6561-423a-8bfc-787980ce421f"},
+            {"action": "upgrade-format-version", "format-version": 1},
+            {
+                "action": "add-schema",
+                "schema": {
+                    "type": "struct",
+                    "fields": [
+                        {"id": 1, "name": "id", "type": "int", "required": False},
+                        {"id": 2, "name": "data", "type": "string", "required": False},
+                    ],
+                    "schema-id": 0,
+                    "identifier-field-ids": [],
+                },
+                "last-column-id": 2,
+            },
+            {"action": "set-current-schema", "schema-id": -1},
+            {"action": "add-spec", "spec": {"spec-id": 0, "fields": []}},
+            {"action": "set-default-spec", "spec-id": -1},
+            {"action": "add-sort-order", "sort-order": {"order-id": 0, "fields": []}},
+            {"action": "set-default-sort-order", "sort-order-id": -1},
+            {"action": "set-location", "location": "s3://warehouse/database/table"},
+            {"action": "set-properties", "updates": {"owner": "bryan", "write.metadata.compression-codec": "gzip"}},
+        ],
+    }
+    assert actual_response == expected
+
+
 def test_create_table_409(rest_mock: Mocker, table_schema_simple: Schema) -> None:
     rest_mock.post(
         f"{TEST_URI}v1/namespaces/fokko/tables",