kevinjqliu commented on code in PR #977: URL: https://github.com/apache/iceberg-python/pull/977#discussion_r1700805836
########## tests/catalog/test_sql.py: ########## Review Comment: could you also add a test for glue catalog? `_update_and_stage_table` is called by glue as well https://github.com/search?q=repo%3Aapache%2Ficeberg-python%20_update_and_stage_table&type=code ########## tests/table/test_init.py: ########## @@ -1156,3 +1157,43 @@ def test_serialize_commit_table_request() -> None: deserialized_request = CommitTableRequest.model_validate_json(request.model_dump_json()) assert request == deserialized_request + + +def test_update_metadata_log(table_v2: Table) -> None: + new_snapshot = Snapshot( + snapshot_id=25, + parent_snapshot_id=19, + sequence_number=200, + timestamp_ms=1602638593590, + manifest_list="s3:/a/b/c.avro", + summary=Summary(Operation.APPEND), + schema_id=3, + ) + + new_metadata = update_table_metadata( + table_v2.metadata, (AddSnapshotUpdate(snapshot=new_snapshot),), False, table_v2.metadata_location + ) + assert len(new_metadata.metadata_log) == 2 + + +def test_update_metadata_log_overflow(table_v2: Table) -> None: + metadata_log = [ + MetadataLogEntry( + timestamp_ms=1602638593590 + i, + metadata_file=f"/path/to/metadata/{i}.json", + ) + for i in range(10) + ] + table_v2.metadata = table_v2.metadata.model_copy(update={"metadata_log": metadata_log, "last_updated_ms": 1602638593600}) + table_v2.metadata_location = "/path/to/metadata/10.json" + assert len(table_v2.metadata.metadata_log) == 10 + + base_metadata = table_v2.metadata + new_metadata = update_table_metadata( + base_metadata, + (SetPropertiesUpdate(updates={"write.metadata.previous-versions-max": "5"}),), Review Comment: nit: perhaps also test 0/negative ########## pyiceberg/table/__init__.py: ########## @@ -1170,14 +1174,18 @@ def _( def update_table_metadata( - base_metadata: TableMetadata, updates: Tuple[TableUpdate, ...], enforce_validation: bool = False + base_metadata: TableMetadata, + updates: Tuple[TableUpdate, ...], + enforce_validation: bool = False, + metadata_location: Optional[str] = None, Review Comment: I see, thanks for the clarification! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
