[rabbitmq-discuss] FW: Multiple consumers

[Ben Hood]

Matthias,

I've reworked your comments into this patch, which I've responded to
inline. In essence, the amqp_client module has been thinned and
de-parameterized and I've introduced the modules amqp_connection and
amqp_channel.

BTW, since this is get quite patchy, I've started to use a patch stack
to keep things maintainable and separated, so you can have any patch
as a diff of the vanilla 1.1.0 tree or as a patch series, whichever
you'd prefer.

> I am not convinced parameterised modules are the way to go. You can
> tell that something isn't quite right by the large number of places
> amqp_client has different branches for the network and direct case.
--snip--
> when I think it should be
>
>     {ok, Connection} = amqp_network_client:start(User, Password),
>     Channel = amqp_connection:open_channel(Connection, ChannelNumber, ""),
>     Ticket = amqp_channel:access_request(Channel, Realm)
>     ...
>     amqp_channel:close(Channel),
>     amqp_connection:close(Connection).
>
> The module parameterisation and general client module structure is
> driven largely by the need to support both the direct and network
> client. I reckon we are trying to bite off too much in one go
> here, and suggest concentrating on the network client first.

I've removed the parameterization so the user code looks like this:

{ok, Connection} = amqp_connection:start(User, Password, Host),
Channel = amqp_connection:open_channel(Connection, ChannelNumber, ""),
amqp_channel:access_request(Channel, Realm),
%%...
amqp_channel:stop(Channel),
amqp_connection:stop(Connection).

This differs only marginally from your suggestion in that I don't use
a separate amqp_network_client module, because when I did so, I
realized that this was very thin and in actual fact quite tied to the
amqp_connection module.

Part of the root cause for the parameterization was the direct/network
branching in the amqp_client akin to ifdef macros in C. I've reduced
the use of these and made them dependent on an internal flag, so they
haven't gone away (yet). Whether they should completely disappear is a
matter of design - amqp_send could be implemented in a specific driver
rather than the generic client, but I think that may be 6 of one and
half a dozen of the other. I think this is something we need to
discuss.

> 2)
> directly. E.g. instead of
>
>     #'queue.declare_ok'{queue = Q1,
>                         message_count = MessageCount,
>                         consumer_count = ConsumerCount}
>                         = AMQPClient:queue_declare(Channel, Ticket, Q)
>
> you'd write
>
>     #'queue.declare_ok'{queue = Q1,
>                         message_count = MessageCount,
>                         consumer_count = ConsumerCount}
>                         = amqp_channel:rpc(
>                             #'queue.declare'{ticket = Ticket,
>                                              queue = binary(Q)}),

I'll look into this as another patch in the stack, with the 2nd
highest priority.

> Why does amqp_client use an ets table instead of a record to record the
> username, password etc?

I'll look into this as another patch in the stack next as this is low
hanging fruit.

Ben
-------------- next part --------------
A non-text attachment was scrubbed...
Name: client_patch_series-3.patch
Type: application/octet-stream
Size: 56018 bytes
Desc: not available
Url : http://lists.rabbitmq.com/pipermail/rabbitmq-discuss/attachments/20070807/b2a38b77/attachment.obj