search

starburstdata / metabase-driver

Starburst Metabase driver
Apache License 2.0
65 stars 10 forks source link

Support for new datetime functions #59

Closed Maiquu closed 1 year ago

Maiquu commented 1 year ago

Adds support for the new datetime functions introduced in Metabase 0.45 and a small fix related to detection of types that can be suffixed with AT TIME ZONE.

Metabase 0.45 blog post

andrewdibiasio6 commented 1 year ago

Looks like the tests have failed. Were you able to run it locally?

I wonder if bumping to "v1.45.0" caused some tests to fail.

Tests off of main look good.

Were you able to execute the tests locally?

andrewdibiasio6 commented 1 year ago

in unreleased, please add bullets to highlight your changes, especially the version bump.

Maiquu commented 1 year ago

I admit I haven't tested it properly at local. So I need a bit more time to get familiar with the test environment. At the moment I managed to make the driver tests for the current version pass locally. (with Metabase at v1.44.0, Driver at v1.0.6) I will keep testing and try to eliminate issues one by one on my end before triggering CI again.

Maiquu commented 1 year ago

Also bumping metabase to v1.45.0 throws around 378 errors in tests with the main version so might need to see to those as well. I'll see if the failed tests are new ones or modified in any way.

Maiquu commented 1 year ago

Should be ready for review/tests.

hovaesco commented 1 year ago

CI accepted

andrewdibiasio6 commented 1 year ago

I'll review shortly, but you will need to squash some of the commits. I think anything you did related to bumping to 1.45, including fixing the tests etc., should just be squashed into one commit.

Great job getting the CI passing though.

Maiquu commented 1 year ago

Played around with commits and trimmed them down. File changes are all same except CHANGELOG.md. All test fixes related to bumping metabase version should be in a single commit now.

andrewdibiasio6 commented 1 year ago

👀