[
https://issues.apache.org/jira/browse/QPID-5615?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13981621#comment-13981621
]
Rob Godfrey commented on QPID-5615:
-----------------------------------
Alex,
Thanks for all your review comments, I will commit changes addressing most of
your comments presently... however the following I am not currently planning on
addressing as part of this JIRA / am addressing in a different way:
{quote}
* If validation fails in any of configured object, it looks like in Management
mode we will not be able to start the broker anymore....
{quote}
This is true, and I think it relates to the general issue of what to do with
objects that fail validation... and how we deal with things in an "errored"
state. I think this problem needs an overall cohesive solution rather than a
patch to just restore the previous functionality for management mode.
{quote}
* Port implementations perhaps need to have a code restricting protocol changes
to only supported protocols. For instance, changing of port protocols to AMQP
protocols on HTTP port should not be allowed. It seems like it is allowed now
via REST api. I am not sure if it is an existing issue.
{quote}
Yes - I think the overall model of ports needs work... I think that AMQP, HTTP,
JMX and RMI ports are (currently) fundamentally different things, and modelling
them using the same object makes little sense. If we could run HTTP and AMQP
on the same port then it would make sense to have both these things be "ports"
but I'm not sure we'd ever be running JMX on the same port as this. I think it
would make more sense to move the JMX port assignments to be attributes on the
JMX Management plugin.
{quote}
* org.apache.qpid.server.model.BrokerModel.getInstance() IMHO, we can stop
using singleton pattern for this. In broker code it called only once on the
broker start-up in org.apache.qpid.server.Broker.startupImpl(BrokerOptions)
{quote}
Actually i think this is a rare case where a singleton is a sensible pattern to
use. If we allowed for multiple instances then every instance would be the
same, and immutable. The important thing is to not rely on the Model being
passed actually being a "BrokerModel" in any of the rest of the code
{quote}
Dead Code:
*
org.apache.qpid.server.model.adapter.FileSystemPreferencesProviderImpl.openNewStore()
{quote}
Not dead - used by the setter after the _path is set. I've added a comment
above the method to this effect.
{quote}
*
org.apache.qpid.server.model.AbstractConfiguredObject.AbstractConfiguredObject(Map<Class<?
extends ConfiguredObject>, ConfiguredObject<?>>, Map<String, Object>)
{quote}
I've moved everything apart from VirtualHostNodes and SystemContext move to use
this constructor (i.e. to inherit the task executor of their parent). At these
two points in the tree we probably want to have a new task executor created
(although we don't currently do this at the VHN).
{quote}
*
org.apache.qpid.server.management.plugin.servlet.rest.action.ListAccessControlProviderAttributes.perform(Map<String,
Object>, Broker) contains the commented code with TODO RG - fix.
{quote}
Yes - this will need fixing at a later date. I think the right approach for
this is not for the broker code to be supplying attribute names/descriptions
through a servlet, but for each type to have its own html / js files. The Web
UI is a plugin and the broker shouldn't really have code in it specifically to
support the nature of the plugin.
{quote}
Not essential issues in a temporary code:
{quote}
As these are in code we know we want to kill, I've not addressed these
> [Java Broker] Broker and VirtualHost should use the same API for
> configuration stores
> -------------------------------------------------------------------------------------
>
> Key: QPID-5615
> URL: https://issues.apache.org/jira/browse/QPID-5615
> Project: Qpid
> Issue Type: Sub-task
> Components: Java Broker
> Reporter: Rob Godfrey
> Assignee: Rob Godfrey
>
> Currently there are two different interfaces for the persisting of configured
> object, one which is used by objects that live directly under the broker, and
> one which is used by objects underneath the virtual host.
> The two APIs should be unified, the recovery process should me made generic,
> and a standardised way of differentiating between objects which inherit their
> parents store and those which manage their own should be made (Broker and
> VirtualHost should not be special-cased in any store logic).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
