killme2008 / carmine-sentinel

A Clojure library designed to connect redis by sentinel, make carmine to support sentinel.
Eclipse Public License 1.0
14 stars 4 forks source link

support carmine upper than v2.15.0 #17

Closed ylgrgyq closed 5 years ago

ylgrgyq commented 5 years ago

Some of the refactorings on carmine.commands/defcommand from com.taoensso/carmine v2.15.0 has made every sentinel commands defined in carmine-sentinel invalid (The defcommand in old version is here for reference). We need to update those definitions like follows to support com.taoensso/carmine upper than v2.15.0:

(ns carmine-sentinel.core
  (:require [taoensso.carmine.commands :as cmds]))

(cmds/defcommand "SENTINEL get-master-addr-by-name"
  {:fn-name         "sentinel-get-master-addr-by-name"
   :fn-params-fixed [name]
   :fn-params-more  nil
   :req-args-fixed  ["SENTINEL" "get-master-addr-by-name" name]
   :cluster-key-idx 2
   :fn-docstring    "get master address by master name. complexity O(1)"})

(cmds/defcommand "SENTINEL slaves"
  {:fn-name         "sentinel-slaves"
   :fn-params-fixed [name]
   :fn-params-more  nil
   :req-args-fixed  ["SENTINEL" "slaves" name]
   :cluster-key-idx 2
   :fn-docstring    "get slaves address by master name. complexity O(1)"})

(cmds/defcommand "SENTINEL sentinels"
  {:fn-name         "sentinel-sentinels"
   :fn-params-fixed [name]
   :fn-params-more  nil
   :req-args-fixed  ["SENTINEL" "sentinels" name]
   :cluster-key-idx 2
   :fn-docstring    "get sentinel instances by mater name. complexity O(1)"})

But this is a breaking change which will cause the users who still sticking on com.taoensso/carmine v2.14.0to fail. So maybe we can just record this issue and leave carmine-sentinel as it is. Those who do need to use com.taoensso/carmine upper than v2.15.0 can refer to the codes above and make it work by themselves. 😱

bsless commented 5 years ago

@ylgrgyq done in the latest update

ylgrgyq commented 5 years ago

@bsless 👍 👍 Thanks for the reminding and for the great work! So I think I can close this issue now.