Name change to "-fn" style was not made everywhere #13

Closed
marick opened this Issue Mar 6, 2013 · 1 comment

Comments

Projects
None yet
3 participants
@marick

marick commented Mar 6, 2013

We had the problem that this didn't do anything in response to a rabbitmqctl stop:

    (let [thread (Thread. #(lc/subscribe ch (:queue configuration) handler :auto-ack true
                                         :handle-shutdown-signal (fn [& args] (prn "shutdown caught" args))
                                         :handle-recover-ok (fn [& args] (prn "recover caught" args))))]

The problem is this:

subscribe in consumers.clj has this code:

        consumer  (create-default channel
                                  :handle-delivery-fn f
                                  :handle-consume-ok      (get cons-opts :handle-consume-ok)
                                  :handle-cancel-ok       (get cons-opts :handle-cancel-ok)
                                  :handle-cancel          (get cons-opts :handle-cancel)
                                  :handle-recover-ok      (get cons-opts :handle-recover-ok)
                                  :handle-shutdown-signal (get cons-opts :handle-shutdown-signal))]

Notice that, except for the most important handle-delivery-fn, there's no -fn suffix.

Now consider the declaration of create-default. It looks like this:

(defn ^Consumer create-default
  "Instantiates and returns a new consumer that handles various consumer life cycle events. See also langohr.basic/consume."
  [^Channel channel &{:keys [handle-consume-ok-fn
                             handle-cancel-fn
                             handle-cancel-ok-fn
                             handle-shutdown-signal-fn
                             handle-recover-ok-fn
                             handle-delivery-fn]}]

Notice that it looks for keys ending in -fn. So the :handle-shutdown-signal value is ignored. As a result, here's what happens when an actual shutdown signal comes in:

    (handleShutdownSignal [^String consumer-tag ^ShutdownSignalException sig]
      (when handle-shutdown-signal-fn
        (handle-shutdown-signal-fn consumer-tag sig)))

The when turns the function into a no-op.

I've confirmed that making the following change to subscribe correctly calls the handler function:

                                   :handle-recover-ok      (get cons-opts :handle-recover-ok)
-                                  :handle-shutdown-signal (get cons-opts :handle-shutdown-signal))]
+                                  :handle-shutdown-signal-fn (get cons-opts :handle-shutdown-signal))]
@ifesdjeen

This comment has been minimized.

Show comment Hide comment
@ifesdjeen

ifesdjeen Mar 6, 2013

Collaborator

Thanks for catching that!

I've added the change here: 8802c62

I haven't added tests just yet. Should I release a new version (beta13)?

Collaborator

ifesdjeen commented Mar 6, 2013

Thanks for catching that!

I've added the change here: 8802c62

I haven't added tests just yet. Should I release a new version (beta13)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment