kevinjqliu merged PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385
--
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...@i
Fokko commented on PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#issuecomment-2549561323
@kevinjqliu Thanks, some bad tests. They didn't have the `manifest-list` in
the snapshot, which is invalid according to the spec.
--
This is an automated message from the Apache Gi
kevinjqliu commented on PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#issuecomment-2549553584
`tests/table/test_metadata.py::test_serialize_v1` and
`tests/table/test_metadata.py::test_v1_write_metadata_for_v2` also fails
--
This is an automated message from the Apache
kevinjqliu commented on PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#issuecomment-2549549009
looks like `manifest-list` is missing in our example
https://github.com/apache/iceberg-python/blob/b0ea716c91f19281d3d9cd7b6965d5d01f6cc3d5/tests/conftest.py#L628
--
This i
Fokko commented on code in PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#discussion_r1889157560
##
pyiceberg/table/snapshots.py:
##
@@ -239,9 +239,7 @@ class Snapshot(IcebergBaseModel):
parent_snapshot_id: Optional[int] = Field(alias="parent-snapshot-id"
kevinjqliu commented on code in PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#discussion_r1889038870
##
pyiceberg/table/snapshots.py:
##
@@ -239,9 +239,7 @@ class Snapshot(IcebergBaseModel):
parent_snapshot_id: Optional[int] = Field(alias="parent-snapsho
Fokko commented on PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#issuecomment-2504747642
Then we should read the `manifests` field, but that has never been
implemented on PyIceberg. No engine is really using that, and I think we should
deprecate it. By making the manifes
kevinjqliu commented on PR #1385:
URL: https://github.com/apache/iceberg-python/pull/1385#issuecomment-2504651007
> we can assume that the manifest-list is there all the time.
what about for V1 tables where the `manifest-list` field is optional?
--
This is an automated message from
Fokko opened a new pull request, #1385:
URL: https://github.com/apache/iceberg-python/pull/1385
Since we don't support `manifests`, we can assume that the manifest-list is
there all the time.
https://github.com/user-attachments/assets/3c438a98-318b-4dfd-8624-78996ce17c19";>
--
