Remove Guava usages in main source in favor of JDK built-ins

sky-uk / cqlmigrate

Cassandra schema migration library

BSD 3-Clause "New" or "Revised" License

46 stars 29 forks source link

Remove Guava usages in main source in favor of JDK built-ins #76

Closed mkobit closed 5 years ago

mkobit commented 5 years ago

Notes

Related to https://github.com/sky-uk/cqlmigrate/issues/75

lw346 commented 5 years ago

Thanks for the contribution!

Unfortunately it looks like the Guava version being pulled in is still an older version, and this is causing some test failures. Running ./gradlew dependencies informs me that cassandra-driver-core v3.1.0 -> v3.2.0 upgrades Guava from 16.0.1 to 19.0. I think to avoid problems in the tests, the cassandraunit dependency will also need upgrading.

Personally, I'm very much in favour of getting rid of Guava for precisely this issue - Guava versions have very limited compatibility between each other, and this often causes the ClassNotFound or MethodNotFound exceptions that we see in these tests.

mkobit commented 5 years ago

Thanks @lw346 , I didn't catch the test dependency leaking in.

Do you think it is better to:

Spend a little bit more time trying to fully strip out Guava

Upgrade cassandraunit to get the tests passing for the current changeset that uses the newer non-deprecated versions of CharSet

I'm slightly inclined to do the first

lw346 commented 5 years ago

If you've got the time and willingness, the first is definitely the preferred option :smile:

mkobit commented 5 years ago

This may even be more work, especially with 4.0 driver just released that itself shades Guava.

There are some more uses of Guava that I didn't originally see

like:

https://github.com/sky-uk/cqlmigrate/blob/b6711aebe0dff6d7d6e364ae975267faabcfecdd/src/main/java/uk/sky/cqlmigrate/SchemaUpdates.java#L8-L10

lw346 commented 5 years ago

If you think it's too much effort to remove Guava as part of this PR (versus upgrading dependencies), then don't worry about it - we can always do that in a later PR.

mkobit commented 5 years ago

I was able to replace them with JDK built-ins when built on top of https://github.com/sky-uk/cqlmigrate/pull/77, but I'm having difficulty getting through dependency hell with different Guavas and clashes with scassandra, cassandra-driver-core, and cassandra-unit - hopefully I'll have something better up here by EOD

mkobit commented 5 years ago

Rebased on top of the other merge in #77 and I think I got all of the Guava stuff out of main