You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "tomtongue (via GitHub)" <gi...@apache.org> on 2025/02/21 11:46:32 UTC
[PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
tomtongue opened a new pull request, #12369:
URL: https://github.com/apache/iceberg/pull/12369
The puffin file for DV includes the following additional `Iceberg` in its footer payload before the Iceberg full build version. I believe this is not intentional (if it's intentional, please let me know). This commit removes that additional 'Iceberg'
```
$ cat 00000-5-8c2c8943-ad1c-407a-9e73-7ff393ef06b0-00001-deletes.puffin
PFA1"��9d:0�PFA1{"blobs":[{"type":"deletion-vector-v1","fields":[2147483645],"snapshot-id":-1,"sequence-number":-1,"offset":4,"length":42,"properties":{"referenced-data-file":"s3://gsweep/iceberg-v3-spec/dvpuff/data/00000-3-f7da0f05-7501-4e61-b49a-10444c666cb5-0-00001.parquet","cardinality":"1"}}],"properties":{"created-by":"Iceberg Apache Iceberg 1.7.0-SNAPSHOT (commit d4fe23a154d4324eeb6ef4753f50834dc4804bd7)"}}�PFA1%
```
The footer payload in the following output shows `{"created-by":"Iceberg Apache Iceberg 1.7.0-SNAPSHOT...`.
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12369:
URL: https://github.com/apache/iceberg/pull/12369#discussion_r1965426482
##########
core/src/main/java/org/apache/iceberg/deletes/BaseDVFileWriter.java:
##########
@@ -143,7 +143,7 @@ private void write(PuffinWriter writer, Deletes deletes) {
private PuffinWriter newWriter() {
EncryptedOutputFile outputFile = fileFactory.newOutputFile();
- String ident = "Iceberg " + IcebergBuild.fullVersion();
+ String ident = IcebergBuild.fullVersion();
Review Comment:
Thanks for checking this. I'll reflect the comment.
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12369:
URL: https://github.com/apache/iceberg/pull/12369#discussion_r1965455912
##########
core/src/main/java/org/apache/iceberg/deletes/BaseDVFileWriter.java:
##########
@@ -143,7 +143,7 @@ private void write(PuffinWriter writer, Deletes deletes) {
private PuffinWriter newWriter() {
EncryptedOutputFile outputFile = fileFactory.newOutputFile();
- String ident = "Iceberg " + IcebergBuild.fullVersion();
+ String ident = IcebergBuild.fullVersion();
Review Comment:
@nastra Reflect the comment
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #12369:
URL: https://github.com/apache/iceberg/pull/12369
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on code in PR #12369:
URL: https://github.com/apache/iceberg/pull/12369#discussion_r1965455912
##########
core/src/main/java/org/apache/iceberg/deletes/BaseDVFileWriter.java:
##########
@@ -143,7 +143,7 @@ private void write(PuffinWriter writer, Deletes deletes) {
private PuffinWriter newWriter() {
EncryptedOutputFile outputFile = fileFactory.newOutputFile();
- String ident = "Iceberg " + IcebergBuild.fullVersion();
+ String ident = IcebergBuild.fullVersion();
Review Comment:
@nastra Reflect the comment. Could you review again?
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #12369:
URL: https://github.com/apache/iceberg/pull/12369#discussion_r1965380101
##########
core/src/main/java/org/apache/iceberg/deletes/BaseDVFileWriter.java:
##########
@@ -143,7 +143,7 @@ private void write(PuffinWriter writer, Deletes deletes) {
private PuffinWriter newWriter() {
EncryptedOutputFile outputFile = fileFactory.newOutputFile();
- String ident = "Iceberg " + IcebergBuild.fullVersion();
+ String ident = IcebergBuild.fullVersion();
Review Comment:
nit: you might want to just inline it: `return Puffin.write(outputFile).createdBy(IcebergBuild.fullVersion()).build();`
--
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
Re: [PR] Nit: Remove additional 'Iceberg' in Puffin footer payload [iceberg]
Posted by "tomtongue (via GitHub)" <gi...@apache.org>.
tomtongue commented on PR #12369:
URL: https://github.com/apache/iceberg/pull/12369#issuecomment-2675020710
Thanks so much for the review!! @nastra @ajantha-bhat @Fokko
--
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