moomindani commented on code in PR #3551:
URL: https://github.com/apache/iceberg-python/pull/3551#discussion_r3544176730
##########
pyiceberg/table/metadata.py:
##########
@@ -574,9 +574,6 @@ def construct_refs(self) -> TableMetadata:
next_row_id: int | None = Field(alias="next-row-id", default=None)
"""A long higher than all assigned row IDs; the next snapshot's
`first-row-id`."""
- def model_dump_json(self, exclude_none: bool = True, exclude: Any | None =
None, by_alias: bool = True, **kwargs: Any) -> str:
- raise NotImplementedError("Writing V3 is not yet supported, see:
https://github.com/apache/iceberg-python/issues/1551";)
Review Comment:
One edge case this change surfaces: the v3 spec allows an optional
`encryption-keys` list in table metadata, and the Java `TableMetadataParser`
writes it when present. PyIceberg doesn't model the field, and the pydantic
models ignore unknown fields, so it's dropped at parse time. With serialization
enabled, loading a v3 table that carries `encryption-keys` and committing any
update would rewrite the metadata without the keys, which would break an
encrypted table. Would it make sense to add a passthrough `encryption-keys`
field to `TableMetadataV3` so it round-trips, even before encryption is
actually supported?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
