qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rob Godfrey (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (QPID-5615) [Java Broker] Broker and VirtualHost should use the same API for configuration stores
Date Fri, 25 Apr 2014 21:19:18 GMT

    [ 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: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message