-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade iceberg support to 1.6.1 #12208
base: branch-25.04
Are you sure you want to change the base?
Conversation
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com> Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
b05b5f0
to
c51c5f5
Compare
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
build |
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
build |
cc @gerashegalov Should we disable iceberg integration test first? I tried to run locally and it failed, and it seems not trivial to fix them. |
We can add them back one by one when we resolve issues in #12176 |
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
Signed-off-by: liurenjie1024 <liurenjie2008@gmail.com>
build |
@liurenjie1024 we should not skip the tests if there are known failures that users will be exposed to. |
Hi, @sameerz Do you mean we should add xfail to know failures? |
Hi, @sameerz I've checked the error and the failure reason is that iceberg 1.6.1 no longer support directly creating data source using iceberg, it requires setting up a catalog to do that. If we want to fix it, we need to change all tests, which seems unnecessary effort since we will add them back gradually when we implement features. WDYT? |
We can xfail tests if we create an epic to fix it and reference them in our test if we fix/add a fixture to create proper tables via SQL or catalog api, do we expect most tests to pass with the current code ? |
Closes #12193 .
Upgrade iceberg version to 1.6.1, also skip iceberg tests for now.