[jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans

2006-06-29 Thread John Sisson (JIRA)
[ 
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] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans

2006-06-29 Thread John Sisson (JIRA)
[ 
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] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans

2006-06-29 Thread Jason Dillon (JIRA)
[ 
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



Re: [jira] Commented: (GERONIMO-2135) Improve the ActiveMQ GBeans

2006-06-22 Thread David Jencks
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) dev@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

2006-06-21 Thread Gianny Damour (JIRA)
[ 
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