Remove dependency on airlift units from client #25772
Conversation
wendigo
force-pushed
the
serafin/airlift-jdbc
branch
from
93c68ad to
14133ec
Compare
Since client libraries were moved to JDK 11 it's now possible to use java.time.Duration instead of Airlift's Duration class. Tests are now adjusted to the fact that the smallest duration unit is nanosecond (which is the smallest "measurable" unit in Java anyway). This means that 1B and 1ns are smallest units.
|
What's the purpose of this change? It just seems to make things more complicated. |
|
@martint we have long-standing issue with a mixed dependency versions and pinning. This removes this problem. |
wendigo
force-pushed
the
serafin/airlift-jdbc
branch
from
14133ec to
3f8aa0a
Compare
|
What's the issue? It's the first time I hear about it and haven't seen any PRs or builds fail due to that. |
|
@martint trino-jdbc pulls in old version of units while trino-main depends on the newer one. In the trino-jdbc tests, when you want to bring TestingTrinoServer up with some additional catalog configuration that depends on the Duration or DataSize being in a newer version, this will fail. |
|
We don't need units in the client to make it work. So removing it, removes this dependency mess |
|
There's a more fundamental problem. |
|
@martint yes but since we keep versions in sync across modules (except for units) we are sure that jdbc uses dependencies supporting JDK11+ |
|
That's not guaranteed. If we need to bump a dependency in trino-main, we'll do that, even if that dependency is not compatible with Java 11. |
|
@martint I understand but we are not in this situation right now except for airlift:units which I'm trying to remove in this PR |
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: