search

Zoomdata / edc-cratedb

Apache License 2.0
4 stars 7 forks source link

upgrade the crate jdbc driver to 2.1.1 #12

Closed kovrus closed 7 years ago

kovrus commented 7 years ago

@bowenwr Can you verify that CrateDB >= 0.57.0 is running in the docker container. How does the connection string look like? It must be smth like crate://localhost:5432/

bowenwr commented 7 years ago

@kovrus Running a single node of Crate 0.57.4, EDC is showing:

2016-12-06 15:01:19.200  INFO 15669 --- [tp1268959798-13] c.z.c.e.f.p.GenericSQLDataProvider       : New pool with key SQLConnectionPoolKey(jdbcUrl=crate://localhost:5432, username=null, customParams={})
2016-12-06 15:01:19.203 ERROR 15669 --- [tp1268959798-13] c.z.c.e.f.p.GenericSQLDataProvider       : Connection factory returned null from createConnection

I will grab a SQL client and try to verify the Crate connection independently later today.

kovrus commented 7 years ago

Thanks @bowenwr

crate://localhost:5432 must be crate://localhost:5432/

bowenwr commented 7 years ago

@kovrus Thanks for the help on the JDBC URL. I was able to connect, but running our suite of integration tests the majority fail.

29 OK, 145 FAIL, 0 SKIP
Total score: 16.7%

We'll need to work through the connector to see what changes we need to make.

bowenwr commented 7 years ago

Speaking with @kovrus, it looks like our tests are failing because Crate now returns timestamps as Unix epoch strings while the integration test suite is looking for ISO date strings.

We have a fix that is pending for the test suite to handle this exact case, I will discuss with the team.

bowenwr commented 7 years ago

Hi @kovrus, this should be handled by https://github.com/Zoomdata/edc-cratedb/pull/14 if you approve of the changes there.

bowenwr commented 7 years ago

Done in scope of dev/upgrade-crate-and-driver, already merged. Thanks for the contribution!