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 2020/12/17 21:22:47 UTC

[GitHub] [iceberg] shardulm94 opened a new issue #1954: Avro: Schema conversions should preserve docs

shardulm94 opened a new issue #1954:
URL: https://github.com/apache/iceberg/issues/1954


   In Avro schema conversions referenced below, we should copy field level documentation between Iceberg and Avro schemas. https://github.com/apache/iceberg/blob/9f727cb366103ea8fed10c1d578a88aa7fbd704f/core/src/main/java/org/apache/iceberg/avro/SchemaToType.java#L31 and https://github.com/apache/iceberg/blob/9f727cb366103ea8fed10c1d578a88aa7fbd704f/core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java#L34


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] kbendick commented on issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
kbendick commented on issue #1954:
URL: https://github.com/apache/iceberg/issues/1954#issuecomment-751536497


   It seems my patch has inadvertently broken a different test (more info in the PR summary).
   
   AFAIK I am unable to revert my PR to a draft as I don't have commit rights in this repo.
   
   I've updated the PR to have [WIP] instead while I investigate.


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] kbendick commented on issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
kbendick commented on issue #1954:
URL: https://github.com/apache/iceberg/issues/1954#issuecomment-751527915


   @shardulm94 @RussellSpitzer I can take this one.


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] shardulm94 commented on issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
shardulm94 commented on issue #1954:
URL: https://github.com/apache/iceberg/issues/1954#issuecomment-755857658


   @kbendick Saw that your PR was merged. Thanks for working on this!


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] shardulm94 closed issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
shardulm94 closed issue #1954:
URL: https://github.com/apache/iceberg/issues/1954


   


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] kbendick commented on issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
kbendick commented on issue #1954:
URL: https://github.com/apache/iceberg/issues/1954#issuecomment-751593152


   I figured out why the test needed to be updated (slight increase in manifest file size due to the field docs appearing in their header). Given the increase is many orders of magnitude less than the default target manifest file size (698 bytes increase per GenericManifestFile according to my debugger from this patch), I personally don't think it's an issue.
   
   This PR is no longer considered WIP.


----------------------------------------------------------------
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.

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


[GitHub] [iceberg] kbendick edited a comment on issue #1954: Avro: Schema conversions should preserve docs

Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on issue #1954:
URL: https://github.com/apache/iceberg/issues/1954#issuecomment-751593152


   I figured out why the test needed to be updated (slight increase in manifest file size due to the field docs appearing in their header). Given the increase is many orders of magnitude less than the default target manifest file size (698 bytes increase per GenericManifestFile according to my debugger from this patch), I personally don't think it's an issue. I've left a comment on the PR in case anybody wants to weigh in on the slight size increase.
   
   This PR is no longer considered WIP.


----------------------------------------------------------------
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.

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