You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/03/24 19:23:44 UTC

[GitHub] [activemq-artemis] jbertram opened a new pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config

jbertram opened a new pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain edited a comment on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-604124890
 
 
   isnt the overloaded createQueue a legacy of pre 2.x model, where queues and topics were modelled differently and thus to keep some compatibilty and for JMS server bits that were still about, the createQueue was more for JMS create a JMS queue where queue and address did match.
   
   Now the question i would ask, is should you be able to make a queue without an address, to me it seems we shouldnt so if anything we should probably clean out that createQueue method that creates and address based on queueName , createQueue really must/should provide an address IMO, and error if not.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-613647549
 
 
   this needs to be rebased.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram opened a new pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram opened a new pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-604124890
 
 
   isnt the overloaded createQueue a legacy of pre 2.x model, where queues and topics were modelled differently and thus to keep some compatibilty and for JMS server bits that were still about, the createQueue was more for JMS create a JMS queue where queue and address did match.
   
   Now the question i would ask, is should you be able to make a queue without an address, to me it seems we shouldnt so if anything we should probably clean out that createQueue method that creates and address based on queueName , createQueue really must/should provide an address IMO.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#discussion_r397599259
 
 

 ##########
 File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java
 ##########
 @@ -47,6 +49,22 @@ public void testStartWithDuplicateQueues() throws Exception {
       }
    }
 
+   @Test
+   public void testQueueWithoutAddressName() throws Exception {
+      final SimpleString QUEUE_NAME = RandomUtil.randomSimpleString();
+      ActiveMQServer server = createServer(false, createDefaultInVMConfig());
+      try {
+         server.getConfiguration().addQueueConfiguration(new CoreQueueConfiguration().setName(QUEUE_NAME.toString()));
 
 Review comment:
   how would that look like on the xml? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-613652196
 
 
   This is moot now that the `QueueConfiguration` work has been merged.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-613653044
 
 
   Actually, the test is still worth merging...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram closed pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram closed pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-603856438
 
 
   @michaelandrepearce, currently if you add a `CoreQueueConfiguration` object with no address to `Configuration` and start the broker the queue will fail to load with a WARN message. However, if you invoke one of the overloaded `createQueue` methods with effectively no address the queue will be created using the queue's name as the address name. The behavior from `createQueue` seems OK to me so I think using `CoreQueueConfiguration` should work the same.
   
   To be clear, the use-case here is an embedded broker without XML configuration.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-605368361
 
 
   fair enough, im not heavily against so go ahead and merge if others also happy, just really doesnt make much sense to have a queue but not to define an address.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-613657590
 
 
   @michaelandrepearce, for what it's worth as I was going through the tests to update them to use the new `createQueue` API I noticed that the same address and queue name are used in hundreds of places so defaulting the address name to the queue name is a nice little usability tweak.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-604166543
 
 
   JMS queues and JMS topics have always been modeled essentially the same way. Back in Artemis 1.x and in HornetQ we had a specific JMS management API and a specific core management API and even then in the core management API we would default the address name to the queue name if the address name was `null`. It wasn't a JMS thing. It was the chosen default behavior for core, and was even advertised [via JavaDoc](http://activemq.apache.org/components/artemis/documentation/javadocs/javadoc-1.5.3/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.html#deployQueue-java.lang.String-java.lang.String-java.lang.String-). I still think it's a reasonable behavior, and I also think changing it could potentially break existing users.
   
   It's also worth noting that the overloaded `createQueue` method which does this is the one that is always ultimately called. In other words, it doesn't matter which `createQueue` method you invoke, if you pass an effectively `null` address name then the queue name will be used.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain edited a comment on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-605368361
 
 
   fair enough, im not heavily against so go ahead and merge if others also happy, just really doesnt make much sense to have a queue but not to have to define an address, its like having a house, but no road leading to it....
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#issuecomment-603685919
 
 
   Surely if a queue has no address then its invalid configuration, shouldnt the loading of the config error...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config

Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3047: ARTEMIS-2680 use q name for addr if none specified via config
URL: https://github.com/apache/activemq-artemis/pull/3047#discussion_r397832801
 
 

 ##########
 File path: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java
 ##########
 @@ -47,6 +49,22 @@ public void testStartWithDuplicateQueues() throws Exception {
       }
    }
 
+   @Test
+   public void testQueueWithoutAddressName() throws Exception {
+      final SimpleString QUEUE_NAME = RandomUtil.randomSimpleString();
+      ActiveMQServer server = createServer(false, createDefaultInVMConfig());
+      try {
+         server.getConfiguration().addQueueConfiguration(new CoreQueueConfiguration().setName(QUEUE_NAME.toString()));
 
 Review comment:
   I don't think this would be possible via the XML. It's only possible by using the `CoreQueueConfiguration` object directly.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services