apache / parquet-format

Apache Parquet Format
https://parquet.apache.org/
Apache License 2.0
1.76k stars 428 forks source link

PARQUET-2369: Clarify GZIP Support for Multiple GZIP Members #218

Closed tustvold closed 10 months ago

tustvold commented 11 months ago

See https://github.com/apache/parquet-testing/pull/41 for more context

tustvold commented 11 months ago

Posted to the mailing list - https://lists.apache.org/thread/zzdh20j8fhrrt3dmx6ow4dbbccmvh277

emkornfield commented 11 months ago

+1 from me.

amassalha commented 11 months ago

Posted to the mailing list - https://lists.apache.org/thread/zzdh20j8fhrrt3dmx6ow4dbbccmvh277

@tustvold I don't see much progress in the mailing list, do you have a deadline for waiting for responds on it and moving forward?

shangxinli commented 11 months ago

LGTM, thanks for working on it.

amassalha commented 11 months ago

+1 from me. @emkornfield not sure if this what holds this merge but your review is still requested, can you please click "approve" if you have no comments?

wgtmac commented 11 months ago

+1 from me. @emkornfield not sure if this what holds this merge but your review is still requested, can you please click "approve" if you have no comments?

I will merge this if there is no objection by the end of this week.

amassalha commented 10 months ago

@wgtmac are you planning to merge? I see all required approved were achieved

wgtmac commented 10 months ago

@wgtmac are you planning to merge? I see all required approved were achieved

I thought there is a minor suggestion: https://github.com/apache/parquet-format/pull/218#discussion_r1382431016. Would you mind adding that?

amassalha commented 10 months ago

@wgtmac are you planning to merge? I see all required approved were achieved

I thought there is a minor suggestion: #218 (comment). Would you mind adding that?

@emkornfield @wgtmac the suggesed link (adding https://datatracker.ietf.org/doc/html/rfc1952#section-2.2 instead of just https://tools.ietf.org/html/rfc1952) , leads to same page as the exists link https://tools.ietf.org/html/rfc1952 seems that this tools.ietf.org site doesn't provide an option to link to a spesific page/section so I don't see a point to duplicate the link

pitrou commented 10 months ago

seems that this tools.ietf.org site doesn't provide an option to link to a spesific page/section so I don't see a point to duplicate the link

Agreed, let's not overdo this.

emkornfield commented 10 months ago

seems that this tools.ietf.org site doesn't provide an option to link to a spesific page/section so I don't see a point to duplicate the link

Agreed, let's not overdo this.

I'm ok with this.

wgtmac commented 10 months ago

I just merged this. Thanks everyone!