search

pingles / clj-hector

Simple Cassandra client for Clojure
42 stars 19 forks source link

Upgrade clojure version #25

Closed nickmbailey closed 12 years ago

nickmbailey commented 12 years ago

We are on 1.2.1 right now. The tests right now break on 1.3. Almost certainly due to the auto boxing behavior changes in 1.3. There might be some issues with any dynamic vars we have as well.

Also 1.4 is out, which again changes boxing behavior. Ints will box to Long in 1.3 but integer in 1.4 believe. We should probably just upgrade straight to 1.4.

pingles commented 12 years ago

Sounds like a plan- have some time tomorrow so might give it a go, as you say- looks like just some numeric behaviour has changed.

pingles commented 12 years ago

So, @nickmbailey I've done some investigating- I think the problem is down to the numeric handling. Specifically, it looks like when Hector serialises the arguments the IntegerSerializer throws a class cast exception.

I'm in 2 minds with what to do:

1) We change the tests so that we specify long/type inferring serialisation. 2) We would need to wrap the serialisation to ensure that the arguments were explicitly converted before being pumped into the serializer.

I'm leaning towards #1- it means less funky code (we're not doing any additional wrapping etc.) but it means that anyone serialising as integers in 1.2 is likely to have class cast exceptions when trying to store values without explicitly converting themselves.

Thoughts?

In case #1 is good I've pushed changes onto a branch (with commit 3ac454821abe2815e5a317a3402d3f318acf7d39) ready to be merged back to master :)

nickmbailey commented 12 years ago

1 seems fine to me at least for now. I mean doesn't 1.2 have the same problem except in reverse? If we used long serializer it would get passed ints and break?

Some custom wrapper for serializing things before they get to hector would be nice at some point but I don't see any reason to add it now.

pingles commented 12 years ago

I've merged the test changes into master and pushed a 0.2.1 release to Clojars- we're now building against Clojure 1.4.0.

nickmbailey commented 12 years ago

@pingles, sorry for the late reply. I've been out of the country for two weeks. The changes from option 1 should be fine for now I think. I'll probably make a ticket for either adding documentation about integers so people aren't confused or maybe looking into something for option 2.