You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by "Hiram Chirino (JIRA)" <de...@geronimo.apache.org> on 2006/06/19 04:32:29 UTC
[jira] Created: (GERONIMO-2135) Improve the ActiveMQ GBeans
Improve the ActiveMQ GBeans
---------------------------
Key: GERONIMO-2135
URL: http://issues.apache.org/jira/browse/GERONIMO-2135
Project: Geronimo
Type: Improvement
Security: public (Regular issues)
Components: ActiveMQ
Reporter: Hiram Chirino
Assigned to: Hiram Chirino
Fix For: 1.2
Suggestions by David Jencks:
I think that this gbean adaptation code should be in geronimo rather
than amq. I'm OK with applying it as is but would prefer some issues
to be addressed first or, even better, immediately after the
transfer (assuming it is done with svn mv).
1. DataSourceReference should be replaced by the geronimo class that
does the same thing, ConnectionFactorySource.
2. I think it would be preferable to get the module/configuration
classloader in the constructor as a magic attribute and use it in
BrokerServiceGBeanImpl.doStart rather than the classloader of
BrokerServiceGBeanImpl.
3. Same for TransportConnectorGBeanImpl.
4. This is a question, not really an issue, about this code:
+ protected TransportConnector createBrokerConnector(String url)
throws Exception {
+ return brokerService.getBrokerContainer().addConnector(url);
+ }
To me it seems like this code is combining the functions of factory
object and container. Is this necessary and appropriate? I'd be
more comfortable with
Connector connector = ConnectorFactory.createConnector(url);
brokerService.getBrokerContainer().addConnector(connector);
I find that the combination style typically creates problems whenever
trying to extend stuff, say by wrapping the connector. What do you
think?
5. hardcoding the protocols in ActiveMQManagerGBean seems like a
temporary expedient at best.
6. javadoc on public JMSConnector addConnector( ... in the manager
gbean seems wrong... does not appear to return an object name.
7. Typo and innaccuracies in the first package.html... this stuff is
only going to work in geronimo, jsr77/88 is not enough.
8. I'm not sure exactly what our official policy is but I prefer to
remove "public" from methods in interfaces since it is the only
choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
Re: [jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by David Jencks <da...@yahoo.com>.
This patch addresses my major concerns. I consider it
a bug fix and thus not requiring further voting beyond
my previous +1, but in case anyone disagrees, I've
applied it and tested it to the best of my abilities
and vote +1.
thanks
david jencks
--- "Gianny Damour (JIRA)" <de...@geronimo.apache.org>
wrote:
> [
>
http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12417237
> ]
>
> Gianny Damour commented on GERONIMO-2135:
> -----------------------------------------
>
> I had a look to the patch and vote +1 for it. Some
> more details:
> 1. is fixed.
> 2. is not fixed.
> 3. is partially fixed
> 4. is not fixed
> 5., 6. and 7. do not know
> 8. is fixed. Also, it seems that we also avoid
> "abstract" from methods in interfaces (one occurence
> in BrokerServiceGBean).
>
>
> > Improve the ActiveMQ GBeans
> > ---------------------------
> >
> > Key: GERONIMO-2135
> > URL:
> http://issues.apache.org/jira/browse/GERONIMO-2135
> > Project: Geronimo
> > Type: Improvement
> > Security: public(Regular issues)
> > Components: ActiveMQ
> > Reporter: Hiram Chirino
> > Assignee: Hiram Chirino
> > Fix For: 1.2
> > Attachments: GERONIMO-2135.patch
> >
> > Suggestions by David Jencks:
> > I think that this gbean adaptation code should be
> in geronimo rather
> > than amq. I'm OK with applying it as is but would
> prefer some issues
> > to be addressed first or, even better,
> immediately after the
> > transfer (assuming it is done with svn mv).
> > 1. DataSourceReference should be replaced by the
> geronimo class that
> > does the same thing, ConnectionFactorySource.
> > 2. I think it would be preferable to get the
> module/configuration
> > classloader in the constructor as a magic
> attribute and use it in
> > BrokerServiceGBeanImpl.doStart rather than the
> classloader of
> > BrokerServiceGBeanImpl.
> > 3. Same for TransportConnectorGBeanImpl.
> > 4. This is a question, not really an issue, about
> this code:
> > + protected TransportConnector
> createBrokerConnector(String url)
> > throws Exception {
> > + return
>
brokerService.getBrokerContainer().addConnector(url);
> > + }
> > To me it seems like this code is combining the
> functions of factory
> > object and container. Is this necessary and
> appropriate? I'd be
> > more comfortable with
> > Connector connector =
> ConnectorFactory.createConnector(url);
> >
>
brokerService.getBrokerContainer().addConnector(connector);
> > I find that the combination style typically
> creates problems whenever
> > trying to extend stuff, say by wrapping the
> connector. What do you
> > think?
> > 5. hardcoding the protocols in
> ActiveMQManagerGBean seems like a
> > temporary expedient at best.
> > 6. javadoc on public JMSConnector addConnector(
> ... in the manager
> > gbean seems wrong... does not appear to return an
> object name.
> > 7. Typo and innaccuracies in the first
> package.html... this stuff is
> > only going to work in geronimo, jsr77/88 is not
> enough.
> > 8. I'm not sure exactly what our official policy
> is but I prefer to
> > remove "public" from methods in interfaces since
> it is the only
> > choice and implied.
>
> --
> This message is automatically generated by JIRA.
> -
> If you think it was sent incorrectly contact one of
> the administrators:
>
>
http://issues.apache.org/jira/secure/Administrators.jspa
> -
> For more information on JIRA, see:
> http://www.atlassian.com/software/jira
>
>
[jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "Gianny Damour (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12417237 ]
Gianny Damour commented on GERONIMO-2135:
-----------------------------------------
I had a look to the patch and vote +1 for it. Some more details:
1. is fixed.
2. is not fixed.
3. is partially fixed
4. is not fixed
5., 6. and 7. do not know
8. is fixed. Also, it seems that we also avoid "abstract" from methods in interfaces (one occurence in BrokerServiceGBean).
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "John Sisson (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12418544 ]
John Sisson commented on GERONIMO-2135:
---------------------------------------
Comment from djencks on dev list 22nd June ( http://www.mail-archive.com/dev%40geronimo.apache.org/msg25153.html )
This patch addresses my major concerns. I consider it
a bug fix and thus not requiring further voting beyond
my previous +1, but in case anyone disagrees, I've
applied it and tested it to the best of my abilities
and vote +1.
thanks
david jencks
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Resolved: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "Hiram Chirino (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=all ]
Hiram Chirino resolved GERONIMO-2135:
-------------------------------------
Resolution: Fixed
thanks for the review guys. patch is applied now.
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "Jason Dillon (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12418550 ]
Jason Dillon commented on GERONIMO-2135:
----------------------------------------
+1 *if* m2 pom's are also updated.
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "John Sisson (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=comments#action_12418545 ]
John Sisson commented on GERONIMO-2135:
---------------------------------------
Comment from hiram on dev list 19th June ( http://www.mail-archive.com/dev%40geronimo.apache.org/msg24939.html) prior to Gianny's Comment above.
Hi folks,
Looking for 3 +1s for the patch attached to
http://issues.apache.org/jira/browse/GERONIMO-2135.
This patch addresses previous concerns raised by David Jencks.
Items not addressed by the patch are:
#4: yes. I agree, but since these GBeans are just meant to be a simple
integration between Geronimo and ActiveMQ, we are taking some short
cuts. In general ActiveMQ allows you to configure many more options
than what the current GBeans allows you to do.
#5, I also agree, but I don't have a programmatic way of listing all
protocols since they are loaded by searching the classpath for
META-INF resources.
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (GERONIMO-2135) Improve the ActiveMQ GBeans
Posted by "Hiram Chirino (JIRA)" <de...@geronimo.apache.org>.
[ http://issues.apache.org/jira/browse/GERONIMO-2135?page=all ]
Hiram Chirino updated GERONIMO-2135:
------------------------------------
Attachment: GERONIMO-2135.patch
Fixes most of the items in this issue.
> Improve the ActiveMQ GBeans
> ---------------------------
>
> Key: GERONIMO-2135
> URL: http://issues.apache.org/jira/browse/GERONIMO-2135
> Project: Geronimo
> Type: Improvement
> Security: public(Regular issues)
> Components: ActiveMQ
> Reporter: Hiram Chirino
> Assignee: Hiram Chirino
> Fix For: 1.2
> Attachments: GERONIMO-2135.patch
>
> Suggestions by David Jencks:
> I think that this gbean adaptation code should be in geronimo rather
> than amq. I'm OK with applying it as is but would prefer some issues
> to be addressed first or, even better, immediately after the
> transfer (assuming it is done with svn mv).
> 1. DataSourceReference should be replaced by the geronimo class that
> does the same thing, ConnectionFactorySource.
> 2. I think it would be preferable to get the module/configuration
> classloader in the constructor as a magic attribute and use it in
> BrokerServiceGBeanImpl.doStart rather than the classloader of
> BrokerServiceGBeanImpl.
> 3. Same for TransportConnectorGBeanImpl.
> 4. This is a question, not really an issue, about this code:
> + protected TransportConnector createBrokerConnector(String url)
> throws Exception {
> + return brokerService.getBrokerContainer().addConnector(url);
> + }
> To me it seems like this code is combining the functions of factory
> object and container. Is this necessary and appropriate? I'd be
> more comfortable with
> Connector connector = ConnectorFactory.createConnector(url);
> brokerService.getBrokerContainer().addConnector(connector);
> I find that the combination style typically creates problems whenever
> trying to extend stuff, say by wrapping the connector. What do you
> think?
> 5. hardcoding the protocols in ActiveMQManagerGBean seems like a
> temporary expedient at best.
> 6. javadoc on public JMSConnector addConnector( ... in the manager
> gbean seems wrong... does not appear to return an object name.
> 7. Typo and innaccuracies in the first package.html... this stuff is
> only going to work in geronimo, jsr77/88 is not enough.
> 8. I'm not sure exactly what our official policy is but I prefer to
> remove "public" from methods in interfaces since it is the only
> choice and implied.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira