taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 130 forks source link

`parse-map` stop working when upgrade from 3.2.0 to 3.3.0 #289

Closed zerg000000 closed 8 months ago

zerg000000 commented 8 months ago

in 3.2.0, it is using 3-arity enc/as-map, but in 3.3.0 it is now using as-map in taoensso.carmine which is 1-arity.

https://github.com/taoensso/carmine/blob/af4bea1f0a1f9fd39d62e8614fd806c7f0be07f5/src/taoensso/carmine.clj#L169

The code using parse-map works in 3.2.0 will failed in 3.3.0 with

(car/wcar {} (car/parse-map (car/hgetall "a") nil (fn [_ v] (parse-long v))))
;; => Wrong number of args (3) passed to: taoensso.carmine/as-map
ptaoussanis commented 8 months ago

@zerg000000 Hi Albert, thanks for the report and sorry about the trouble!

I've just pushed https://github.com/taoensso/carmine/releases/tag/v3.3.1 to Clojars with a hotfix 👍

zerg000000 commented 8 months ago

Thanks for your prompt reply!

zerg000000 commented 8 months ago

@ptaoussanis don't know what's getting wrong

(deftest test-as-map
  (is (= {:a 1 :b 2}
         (as-map ["a" "1" "b" "2"] 
                 (fn [k _] (keyword k)) 
                 (fn [_ v] (parse-long v)))))
  (is (= {"A" "1" "B" "2"}
         (as-map ["a" "1" "b" "2"] 
                 (fn [k _] (.toUpperCase k)))
         "keys should be converted to uppercase"))
  (is (= {:a 1 :b 2}
         (as-map ["a" "1" "b" "2"] :keywordize))
      "keys should be converted to keywords"))

FAIL in baba.processor-test/test-as-map (processor_test.clj:26)
Expected:
  {"A" "1", "B" "2"}
Actual:
  {"A" -"1" +"a", "B" -"2" +"b"}
  -{"A" "1", "B" "2"} +"keys should be converted to uppercase"

FAIL in baba.processor-test/test-as-map (processor_test.clj:30)
keys should be converted to keywords
Expected:
  {:a 1, :b 2}
Actual:
  {+"1" "a", +"2" "b", -:a 1, -:b 2}
ptaoussanis commented 8 months ago

Ack, I had a stupid typo in the hotfix that I somehow missed 🤦

Should actually be fixed now in https://github.com/taoensso/carmine/releases/tag/v3.3.2, really sorry about!!

dharrigan commented 8 months ago

Hi,

Just upgraded to 3.3.1 from 3.3.0 and I now get this:

class taoensso.carmine.protocol.Context cannot be cast to class taoensso.carmine.protocol.Context (taoensso.carmine.protocol.Context is in unnamed module of loader clojure.lang.DynamicClassLoader @2e95d163; taoensso.carmine.protocol.Context is in unnamed module of loader 'app'

Reverting to 3.3.0 for now.

Will try out 3.3.2

ptaoussanis commented 8 months ago

@dharrigan Hi David, that should be unrelated and sounds like you might have stale build artifacts. Please try running lein clean (or equivalent), or clearing out your project's target dir before retrying.

Edit to add for clarity: the typo in 3.3.1 shouldn't affect anyone that wasn't already broken by 3.3.0. I.e. the intended hotfix wasn't successful, but shouldn't introduce any other issues.

dharrigan commented 8 months ago

Interestingly, it builds from a clean CI environment each time.

However, will try. If it persists, I'll raise a separate issue.

-=david=-

ptaoussanis commented 8 months ago

@dharrigan Fwiw I have seen various CI environments do build caching as a performance optimization. But yes, please do create a new issue in case you're still running into trouble!

zerg000000 commented 8 months ago

Thanks for the fix!

dharrigan commented 8 months ago

@ptaoussanis yup, seems like it was a build issue. Unable to reproduce. :-)