-
Notifications
You must be signed in to change notification settings - Fork 68
Update iceberg_to_ducklake
for DuckLake v0.3, add/improve tests
#482
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
Conversation
@Tmonster this is refusing to run any CI, do you know what's going on? |
@Tishj maybe change the base branch to |
We shouldn't, v1.4-andium is where v1.4.0 should be built from, and we will definitely need to keep v1.4-andium around for the LTS, |
I would keep I think proper path is:
|
v1.4-andium is at the moment left behind, and probably should not, but that we can fix tomorrow. |
Ok, I have cancelled the last bit of running CI for now, CI here should run eventually, but potentially is clogged by all other CI on the rest of the extensions. @c-herrewijn does every extension right now have a PR up to bump submodules? I can imagine that is clogging a lot of stuff no? |
I think it would show the runs even if they haven't started, but this doesn't have any CI queued even |
@Tishj I imagine CI passes for you locally? If so, and the builds pass, then I'm fine with merging so we can get this in quick. We can fix the tests in the next PR. edit: Nvm, see you just changed it. No worries then |
@Tishj ping me when you have this! I need to add it to the release post! |
Sure, my CI got canceled for some reason though? |
9afae45
to
6f8e534
Compare
Btw this test is very slow, like, several minutes on a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
Added a test where
iceberg_to_ducklake
is used on the entire generated catalog of tests, and (pretty much) all tables are compared against each other: