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/01/21 09:41:47 UTC
[GitHub] [incubator-iceberg] yujiantao opened a new pull request #743: Bump
Apache spark to 3.0.0-preview2
yujiantao opened a new pull request #743: Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue merged pull request #743: [spark-3] Bump
Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on issue #743: [spark-3] Bump
Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#issuecomment-581183783
Thanks @yujiantao! I caught a couple more minor things. Once those are done, we should be good to merge.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on issue #743: [spark-3]
Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on issue #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#issuecomment-581109895
@rdblue Sorry for late reply as I was on a long vocation. I have refreshed the commit.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373880002
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java
##########
@@ -125,7 +126,7 @@ public void testSparkWriteFailsUnknownTransform() throws IOException {
}
@Test
- public void testSparkStreamingWriteFailsUnknownTransform() throws IOException {
+ public void testSparkStreamingWriteFailsUnknownTransform() throws IOException, TimeoutException {
Review comment:
This should also be `Exception`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373827428
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchWrite.java
##########
@@ -493,6 +494,9 @@ protected void setCurrentKey(PartitionKey currentKey) {
public void write(InternalRow row) throws IOException {
writeInternal(row);
}
+
+ @Override
+ public void close() {}
Review comment:
I will add this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373879959
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestTables.java
##########
@@ -37,7 +37,7 @@
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.LocationProvider;
import org.apache.iceberg.io.OutputFile;
-import parquet.Preconditions;
+import org.apache.parquet.Preconditions;
Review comment:
This should not be using Preconditions from Parquet. It should be the guava one. Can you update it to that one instead?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373827417
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreaming.java
##########
@@ -79,7 +80,7 @@ public static void stopSpark() {
}
@Test
- public void testStreamingWriteAppendMode() throws IOException {
+ public void testStreamingWriteAppendMode() throws IOException, TimeoutException {
Review comment:
Yes, throwing "Exception" is better.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] jerryshao commented on issue #743: [spark-3]
Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
jerryshao commented on issue #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#issuecomment-581282605
LGTM.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373897701
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java
##########
@@ -125,7 +126,7 @@ public void testSparkWriteFailsUnknownTransform() throws IOException {
}
@Test
- public void testSparkStreamingWriteFailsUnknownTransform() throws IOException {
+ public void testSparkStreamingWriteFailsUnknownTransform() throws IOException, TimeoutException {
Review comment:
Ok.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373897701
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java
##########
@@ -125,7 +126,7 @@ public void testSparkWriteFailsUnknownTransform() throws IOException {
}
@Test
- public void testSparkStreamingWriteFailsUnknownTransform() throws IOException {
+ public void testSparkStreamingWriteFailsUnknownTransform() throws IOException, TimeoutException {
Review comment:
Ok
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r370274234
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/SparkBatchWrite.java
##########
@@ -493,6 +494,9 @@ protected void setCurrentKey(PartitionKey currentKey) {
public void write(InternalRow row) throws IOException {
writeInternal(row);
}
+
+ @Override
+ public void close() {}
Review comment:
Can you add a comment to explain why this is empty? If I understand correctly, close is called after `abort` or `commit` is called. Both `abort` and `commit` will close the current data file so there is no more cleanup to do here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on issue #743: [spark-3] Bump
Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#issuecomment-578875422
Thanks for working on this, @yujiantao! Could you fix the test errors? I also had a couple of minor comments.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on issue #743: [spark-3] Bump
Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#issuecomment-581521200
Looks good! Thanks for working on this @yujiantao!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] yujiantao commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
yujiantao commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r373897760
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestTables.java
##########
@@ -37,7 +37,7 @@
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.LocationProvider;
import org.apache.iceberg.io.OutputFile;
-import parquet.Preconditions;
+import org.apache.parquet.Preconditions;
Review comment:
Yes.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request
#743: [spark-3] Bump Apache spark to 3.0.0-preview2
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #743: [spark-3] Bump Apache spark to 3.0.0-preview2
URL: https://github.com/apache/incubator-iceberg/pull/743#discussion_r370273573
##########
File path: spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreaming.java
##########
@@ -79,7 +80,7 @@ public static void stopSpark() {
}
@Test
- public void testStreamingWriteAppendMode() throws IOException {
+ public void testStreamingWriteAppendMode() throws IOException, TimeoutException {
Review comment:
How about throwing `Exception`? Keeping a list of which checked exceptions may be thrown isn't useful for test 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org