You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/09/23 08:28:35 UTC

[GitHub] [iceberg] nastra commented on a diff in pull request #5773: Core: Remove write.manifest-lists.enabled and relevant code paths

nastra commented on code in PR #5773:
URL: https://github.com/apache/iceberg/pull/5773#discussion_r978390263


##########
core/src/main/java/org/apache/iceberg/SnapshotParser.java:
##########
@@ -153,35 +139,22 @@ static Snapshot fromJson(FileIO io, JsonNode node) {
 
     Integer schemaId = JsonUtil.getIntOrNull(SCHEMA_ID, node);
 
-    if (node.has(MANIFEST_LIST)) {

Review Comment:
   Using a fake FileIO in `for (ManifestFile file : snapshot.allManifests(DUMMY_FILE_IO))` sounds great and we don't need expose the `v1ManifestLocations` on the `Snapshot` API itself. I went ahead and implemented that and pushed that to https://github.com/apache/iceberg/pull/5734.
   
   I would suggest that we first finish up https://github.com/apache/iceberg/pull/5734 and then come back to this PR to deal with the `write.manifest-lists.enabled` flag (aka deprecating it and removing it from the write path, but still keeping it in the read path in `SnapshotParser` as you indicated above). I will update this PR here accordingly once https://github.com/apache/iceberg/pull/5734 is in



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org