Github user gemmellr commented on the issue:
https://github.com/apache/qpid-jms/pull/6
Hi Austin,
I think this is a good start. There are some changes that I think would
improve the new example classes, in general I think the classes would benefit
from some simplification in terms of a making a clear messaging example. They
effectively combine to give a request-response example, so for me the main
things are to demonstrate the use of a [Temporary]Queue for responses and using
JMSReplyTo to pass that destination information to the server and for it to use
that in sending responses back.
For example, consider the equivalent Client and Server examples for
Proton's C++ and Python bindings:
https://github.com/apache/qpid-proton/blob/master/examples/python/client.py
https://github.com/apache/qpid-proton/blob/master/examples/python/server.py
https://github.com/apache/qpid-proton/blob/master/examples/cpp/client.cpp
https://github.com/apache/qpid-proton/blob/master/examples/cpp/server.cpp
These look (I haven't actually run them) to just send some fixed messages
with well-known request text, and capitalize them on the server before sending
the responses. I think doing the same here too would make for a clearer
example, showing just the bits people need to know about the client config etc
to get started and the messaging pattern in general, without the added
complexity of commands and reading input expanding the scope.
If we strip the example back to just send/receive basic text messages like
the Proton ones, it should even be possible to run the different examples in
combination fairly easily. That might take a little more work, so I'm not
suggeting you need to do that here, just commenting on it perhaps being a 'nice
to have' some time, and aligning the behaviour more here would make sense as a
starting point to the end. The main complication I can think of in actually
doing it would be that right now the JMS client examples use a queue named
"queue" by default whereas the Proton ones use "examples" by default. The
Proton ones do let you specify the details on the command line, so that would
be one way to align them (the other being updating the JMS clients
jndi.properties file before compiling them). I don't see a need for adding that
complexity here, but I would be happy to change the JMS examples default queue
name config in jndi.properties to match. Again, I'm not suggesting you need to d
o that here though, we can always do that separately later.
Beyond the above, I noted some very small issues looking this over which
would apply even with the simplifications, I'll go put some comments against
the code now.
Robbie
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
