You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by GitBox <gi...@apache.org> on 2022/06/11 00:02:34 UTC
[GitHub] [orc] liujiawinds opened a new pull request, #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
liujiawinds opened a new pull request, #1156:
URL: https://github.com/apache/orc/pull/1156
### What changes were proposed in this pull request?
Extracting the encryption setup logic as a tool class.
### Why are the changes needed?
Currently, the encryption setup at `WriterImpl` is so tightly coupled that the encryption variant cannot be obtained externally. This affects the integration of ORC and Flink.
### How was this patch tested?
It doesn't introduce new features and passed all test cases.
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1152839362
Thank you for making a PR, @liujiawinds .
I set the milestone 1.9.0 which is the version of main branch.
For now, there is no plan of backporting 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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1153545216
After rethinking about this PR, I removed the milestone.
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1153373579
What should I do next for this pr? I don't quite understand the meaning of it being marked as `Changes requested` state.
:)
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1153556056
@dongjoon-hyun The PR description has been revised.
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1152857414
@dongjoon-hyun
> 1. Do you have some documentation or reference in Apache Flink community?
Yes, first we must create a stream based `PhysicalFsWriter` before `WriterImpl` initial. [WriterImpl](https://github.com/apache/orc/blob/7facf81846a5e4395f2dbab335faef93f04c6aee/java/core/src/java/org/apache/orc/impl/WriterImpl.java#L223)
Then, You can look into flink orc writer from [OrcBulkWriterFactory](https://github.com/apache/flink/blob/99c2a415e9eeefafacf70762b6f54070f7911ceb/flink-formats/flink-orc/src/main/java/org/apache/flink/orc/writer/OrcBulkWriterFactory.java#L99).
Given the above, now I want to passing encryption settings to `PhysicalFsWriter` before `WriterImpl` initail, but the encryption setup at `WriterImpl ` is so tightly coupled that the encryption variant cannot be obtained externally.
> 2. Is this Apache ORC design issue?
Yes, I think it is.
> 3. Is this request aligned with other encryption handling in Apache Flink community (like Apache Parquet Modular Encryption handling)?
Parquet has a similar class named `FileEncryptionProperties`. [ParquetWriter](https://github.com/apache/parquet-mr/blob/59e9f78b8b3a30073db202eb6432071ff71df0ec/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java#L283)
>In general, branch-1.8 is already in feature freeze mode. I don't think this is required at v1.8.0 (currently).
It's ok to release in later versions. I will extract this part of code in flink.
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1152869231
Thank you for the details.
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1156: ORC-1200: Extracting encryption setup logic from `WriterImpl`
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1156:
URL: https://github.com/apache/orc/pull/1156#issuecomment-1153544462
Instead of commenting, you are supposed to revise your PR description with your previous comment's content because only the PR title and description becomes a commit message.
> What should I do next for this pr? I don't quite understand the meaning of it being marked as `Changes requested` state.
> :)
--
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@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org