Add iceberg to product tests suite #3092
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. For more information, see https://github.com/prestosql/cla. |
| import static io.prestosql.tests.TestGroups.STORAGE_FORMATS; | ||
| import static io.prestosql.tests.utils.QueryExecutors.onPresto; | ||
|
|
||
| public class TestIcebergCreateTable |
There was a problem hiding this comment.
It would be nice to have some generic connector tests, that we could run for hive and iceberg (and potentially for other connectors too in future).
There was a problem hiding this comment.
The scope of the PR is to add iceberg connector to the product tests, so I limited scope of iceberg tests to a bare minimum (create/delete table and run simple query). I propose deferring generalization between hive and iceberg tests and extending iceberg tests scope to be on par with hive tests to a follow up PRs.
No, it is not and it was not the goal of the PR. The primary focus is to be mostly on par with various HDFS/HMS configurations that hive connector supports also for iceberg connector.
Some tests are expected to fail as iceberg connector does not currently support Kerberized HDFS. I'll exclude iceberg tests from PR checks till support for Kerberized HDFS is added to iceberg connector. |
If iceberg does not support kerberos then it should not be configured in kerberized environments. |
|
It should support Kerberos. We use the same HDFS code and bind everything the same way (at least that was the intention). |
|
Presto iceberg connector does not support Kerberos and the goal of the PR is to provide environments that may be used to fix the iceberg connector. There are a few improvements on the iceberg side that I implemented that will help iceberg connector to work properly with Kerberized HDFS. |
vrozov
force-pushed
the
iceberg-product-tests
branch
from
3fb5f5e to
6bd6b25
Compare
vrozov
force-pushed
the
iceberg-product-tests
branch
from
8159910 to
7f8e691
Compare
vrozov
force-pushed
the
iceberg-product-tests
branch
from
7f8e691 to
58f7391
Compare
vrozov
force-pushed
the
iceberg-product-tests
branch
from
58f7391 to
01e4286
Compare
|
all product tests are fixed, please review. |
| @@ -0,0 +1,15 @@ | |||
| connector.name=iceberg | |||
There was a problem hiding this comment.
I would prefer to not to configure iceberg connector in environments where it does not work. That would be very misleading to other developers, where they would think where something just got broken because it does not work but it is committed.
There was a problem hiding this comment.
IMO, there is a difference between committing broken functionality and providing a test environment that demonstrates that the functionality of a plugin is broken in certain configurations.
Two commits are intentional as the first commit configures iceberg plugin for product tests suite. The second commit disables iceberg plugin in those environments where it will cause automation to fail. |
|
@electrum Please review |
|
@electrum Please take a look |
|
Thanks! |
Fixes #2304