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 2021/04/09 11:45:14 UTC
[GitHub] [iceberg] marton-bod opened a new pull request #2449: Core: Capitalize default write format value
marton-bod opened a new pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449
Following up on https://github.com/apache/iceberg/issues/2440
--
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] pvary commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816654444
Can we add a test case using the default value?
--
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] marton-bod commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816649091
> I swear I saw this PR with capitlizing the property in TableProperties, but I like this approach much better
You are right, that was my initial commit but then I force pushed this second approach :)
--
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] marton-bod commented on pull request #2449: Core: Capitalize default write format value
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816624999
@pvary @aokolnychyi @RussellSpitzer Can you please take a look?
--
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] pvary merged pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
pvary merged pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449
--
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] marton-bod edited a comment on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
marton-bod edited a comment on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816649091
> I swear I saw this PR with capitlizing the property in TableProperties, but I like this approach much better
You are right, that was my initial commit but then I force pushed this second approach :)
Thanks for the review!
--
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] marton-bod commented on a change in pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#discussion_r610633417
##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -662,6 +663,27 @@ public void testMultiTableInsert() throws IOException {
HiveIcebergTestUtils.validateData(target2, target2Records, 1);
}
+ @Test
+ public void testWriteWithDefaultWriteFormat() {
+ Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
Review comment:
That's right, it doesn't add any value to run it for different configurations. I'm going to expand the Assume.
--
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] RussellSpitzer commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816740567
They started cropping up recently ... Not sure what the cause is, perhaps we are failing to turn off a context somewhere? Ie we have 16 Spark Contexts running at this point and then 17 tries to bind and runs out of attempts?
--
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] marton-bod commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816739253
I got lots of flaky spark test failures on my latest CI run here:
```
org.apache.iceberg.spark.source.TestIcebergSourceHiveTables3 > classMethod FAILED
java.net.BindException: Cannot assign requested address: Service 'sparkDriver' failed after 16 retries (on a random free port)! Consider explicitly setting the appropriate binding address for the service 'sparkDriver' (for example spark.driver.bindAddress for SparkDriver) to the correct binding address.
```
Have you seen these before @aokolnychyi @RussellSpitzer ?
--
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] marton-bod commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-816676562
> Can we add a test case using the default value?
Yes, good idea, added a test case.
--
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] pvary commented on pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#issuecomment-817575696
Pushed to master. Thanks for the patch @marton-bod and @RussellSpitzer for the review!
--
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] pvary commented on a change in pull request #2449: Hive: Capitalize write format value when creating writer
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #2449:
URL: https://github.com/apache/iceberg/pull/2449#discussion_r610628235
##########
File path: mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
##########
@@ -662,6 +663,27 @@ public void testMultiTableInsert() throws IOException {
HiveIcebergTestUtils.validateData(target2, target2Records, 1);
}
+ @Test
+ public void testWriteWithDefaultWriteFormat() {
+ Assume.assumeTrue("Tez write is not implemented yet", executionEngine.equals("mr"));
Review comment:
Do we need to run this for every catalog / fileFormat?
Maybe we do not want to create a test class just for this, but at least skip the unnecessary tests.
--
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