You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by GitBox <gi...@apache.org> on 2021/03/31 14:26:20 UTC
[GitHub] [parquet-mr] dossett opened a new pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
dossett opened a new pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886
Make sure you have checked _all_ steps below.
### Jira
- [ ] My PR addresses the following [Parquet Jira](https://issues.apache.org/jira/browse/PARQUET-2012) issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
- https://issues.apache.org/jira/browse/PARQUET-2012
### Tests
- [X] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
--
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
[GitHub] [parquet-mr] gszadovszky merged pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
gszadovszky merged pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886
--
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
[GitHub] [parquet-mr] dossett removed a comment on pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
dossett removed a comment on pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#issuecomment-811109405
cc @belugabehr I know you have at least one other open PR that cleans up parquet-protobuf, just want to make sure this doesn't make things worse.
--
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
[GitHub] [parquet-mr] dossett commented on a change in pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
dossett commented on a change in pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#discussion_r604951366
##########
File path: parquet-protobuf/src/test/java/org/apache/parquet/proto/TestUtils.java
##########
@@ -198,14 +197,11 @@ private static void checkSameBuilderInstance(MessageOrBuilder[] messages) {
* Writes messages to temporary file and returns its path.
*/
public static Path writeMessages(MessageOrBuilder... records) throws IOException {
- return writeMessages(inferRecordsClass(records), records);
- }
-
- public static Path writeMessages(Class<? extends Message> cls, MessageOrBuilder... records) throws IOException {
Path file = someTemporaryFilePath();
+ Class<? extends Message> cls = inferRecordsClass(records);
- ProtoParquetWriter<MessageOrBuilder> writer =
- new ProtoParquetWriter<MessageOrBuilder>(file, cls);
+ ParquetWriter<MessageOrBuilder> writer =
+ ProtoParquetWriter.<MessageOrBuilder>builder(file).withMessage(cls).build();
Review comment:
This change just switches the only internal use of the deprecated constructors to use the `builder` pattern, and does some other minor cleanup with these methods.
--
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
[GitHub] [parquet-mr] dossett commented on pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
dossett commented on pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#issuecomment-811109405
cc @belugabehr I know you have at least one other open PR that cleans up parquet-protobuf, just want to make sure this doesn't make things worse.
--
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
[GitHub] [parquet-mr] dossett commented on a change in pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
dossett commented on a change in pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#discussion_r604952270
##########
File path: parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoParquetWriter.java
##########
@@ -77,26 +81,24 @@ public ProtoParquetWriter(Path file, Class<? extends Message> protoMessage,
* @param file The file name to write to.
* @param protoMessage Protobuf message class
* @throws IOException if there is an error while writing
+ *
+ * @deprecated will be removed in 2.0.0.; Used ProtoParquetWriter.Builder instead
*/
public ProtoParquetWriter(Path file, Class<? extends Message> protoMessage) throws IOException {
this(file, protoMessage, CompressionCodecName.UNCOMPRESSED,
DEFAULT_BLOCK_SIZE, DEFAULT_PAGE_SIZE);
}
-
public static <T> Builder<T> builder(Path file) {
return new Builder<T>(file);
}
public static <T> Builder<T> builder(OutputFile file) {
return new Builder<T>(file);
}
-
private static <T extends MessageOrBuilder> WriteSupport<T> writeSupport(Class<? extends Message> protoMessage) {
- return new ProtoWriteSupport<T>(protoMessage);
+ return new ProtoWriteSupport<>(protoMessage);
Review comment:
Teeny tiny clean-up to remove a redundant type parameter.
--
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
[GitHub] [parquet-mr] gszadovszky commented on a change in pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
gszadovszky commented on a change in pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#discussion_r605496828
##########
File path: parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoParquetWriter.java
##########
@@ -43,6 +43,8 @@
* @param blockSize HDFS block size
* @param pageSize See parquet write up. Blocks are subdivided into pages for alignment and other purposes.
* @throws IOException if there is an error while writing
+ *
+ * @deprecated will be removed in 2.0.0.; Used ProtoParquetWriter.Builder instead
Review comment:
Please, also use the java annotation `@Deprecated` for all the deprecated methods.
--
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
[GitHub] [parquet-mr] dossett commented on a change in pull request #886: PARQUET-2012 Mark ProtoParquetWriter constructors deprecated
Posted by GitBox <gi...@apache.org>.
dossett commented on a change in pull request #886:
URL: https://github.com/apache/parquet-mr/pull/886#discussion_r605620995
##########
File path: parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoParquetWriter.java
##########
@@ -43,6 +43,8 @@
* @param blockSize HDFS block size
* @param pageSize See parquet write up. Blocks are subdivided into pages for alignment and other purposes.
* @throws IOException if there is an error while writing
+ *
+ * @deprecated will be removed in 2.0.0.; Used ProtoParquetWriter.Builder instead
Review comment:
Got it, thanks for spotting that!
--
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