[GitHub] [activemq-nms-amqp] Havret opened a new pull request #32: NO-JIRA: Update docs
Havret opened a new pull request #32: NO-JIRA: Update docs URL: https://github.com/apache/activemq-nms-amqp/pull/32 It removes redundant entry regarding tests. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] cjwmorgan-sol commented on issue #28: NO-JIRA: Extend logging
cjwmorgan-sol commented on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-531410287 I took a look at the qpid jms implementation and it catches Runtime Exceptions not all Exception. The dotnet equivalent would be SystemException. This would allow custom Exceptions to be handled by the connection ExceptionListener just like qpid jms does. Also until the nms provider catches up to qpid jms in terms of their TracableMessage approach application error should not hidden by the provider. Like in the current implementation. Warning logs for application errors seems to be the best middle ground as this follows the Java logging level with qpid jms would have used. Given that there is no other indication for the application failure. @michaelandrepearce since nms does not have a "Trace" level are you suggesting the log level be Debug? Application errors being does not sound right to me. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] cjwmorgan-sol edited a comment on issue #28: NO-JIRA: Extend logging
cjwmorgan-sol edited a comment on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-529578559 I found that when an application callback (Message Listener) throws an exception that is not caught there is no warning level log. Leading to what looks like a dead lock (it was not on using a message had a an issue not the client causing an exception on every message). I needed frame level logging/wire capture plus step into debugging to figure that my application had an issue. When an async delivery fails shouldn't the provider give a Log? (I think it should be warning) @Havret and @michaelandrepearce what you think? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce edited a comment on issue #28: NO-JIRA: Extend logging
michaelandrepearce edited a comment on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-531367659 Why would we log here? Exception here would be because exception was thrown within users message listener code. I would expect if it was important for a user to add their own logging as they see fit into their listener logic. As what people would want would vary. Maybe trace if really felt strongly. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce edited a comment on issue #28: NO-JIRA: Extend logging
michaelandrepearce edited a comment on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-531367659 Why would we log here? Exception here would be because exception was thrown within users message listener code. I would expect if it was important for a user to add their own logging as they see fit into their listener logic. As what people would want would vary 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce commented on issue #28: NO-JIRA: Extend logging
michaelandrepearce commented on issue #28: NO-JIRA: Extend logging URL: https://github.com/apache/activemq-nms-amqp/pull/28#issuecomment-531367659 Why would we log here? Exception here would be because exception was thrown within users message listener code. I would expect if it was important for a user to add their own as they see fit into their listener logic. As what people would want would vary 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce merged pull request #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set
michaelandrepearce merged pull request #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set URL: https://github.com/apache/activemq-nms-amqp/pull/31 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] gemmellr commented on issue #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set
gemmellr commented on issue #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set URL: https://github.com/apache/activemq-nms-amqp/pull/31#issuecomment-531360527 Looks good to me 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2836: [ARTEMIS-2487] Updated to org.jctools:jctools-core:2.1.2
asfgit closed pull request #2836: [ARTEMIS-2487] Updated to org.jctools:jctools-core:2.1.2 URL: https://github.com/apache/activemq-artemis/pull/2836 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce commented on issue #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set
michaelandrepearce commented on issue #31: AMQNET-612: ObjectMessage shouldn't have jms-msg-type set URL: https://github.com/apache/activemq-nms-amqp/pull/31#issuecomment-531356195 @gemmellr i think were all agreed now, but could you just give a +1 that your happy with the change before i merge 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-nms-amqp] michaelandrepearce merged pull request #27: NO-JIRA: Configure continuous integration
michaelandrepearce merged pull request #27: NO-JIRA: Configure continuous integration URL: https://github.com/apache/activemq-nms-amqp/pull/27 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2834: ARTEMIS-2488: Handle the case where source address is null
michaelandrepearce commented on a change in pull request #2834: ARTEMIS-2488:
Handle the case where source address is null
URL: https://github.com/apache/activemq-artemis/pull/2834#discussion_r324272574
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
##
@@ -319,7 +319,7 @@ public void initialise() throws Exception {
if (CompositeAddress.isFullyQualified(source.getAddress())) {
addressToUse =
SimpleString.toSimpleString(CompositeAddress.extractAddressName(source.getAddress()));
queueNameToUse =
SimpleString.toSimpleString(CompositeAddress.extractQueueName(source.getAddress()));
- } else {
+ } else if (source.getAddress() != null) {
Review comment:
An alternate is to use the static method on SimpleString to convert String
to SimpleString which has the null check. Tbh we should probable remove the
public constructor on SimpleString to avoid such issues. E.g. every bit of code
should use the safe static method
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #2834: ARTEMIS-2488: Handle the case where source address is null
michaelandrepearce commented on a change in pull request #2834: ARTEMIS-2488:
Handle the case where source address is null
URL: https://github.com/apache/activemq-artemis/pull/2834#discussion_r324272574
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
##
@@ -319,7 +319,7 @@ public void initialise() throws Exception {
if (CompositeAddress.isFullyQualified(source.getAddress())) {
addressToUse =
SimpleString.toSimpleString(CompositeAddress.extractAddressName(source.getAddress()));
queueNameToUse =
SimpleString.toSimpleString(CompositeAddress.extractQueueName(source.getAddress()));
- } else {
+ } else if (source.getAddress() != null) {
Review comment:
An alternate is to use the static method on SimpleString to convert String
to SimpleString which has the null check.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #2834: ARTEMIS-2488: Handle the case where source address is null
clebertsuconic commented on a change in pull request #2834: ARTEMIS-2488:
Handle the case where source address is null
URL: https://github.com/apache/activemq-artemis/pull/2834#discussion_r324254681
##
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
##
@@ -319,7 +319,7 @@ public void initialise() throws Exception {
if (CompositeAddress.isFullyQualified(source.getAddress())) {
addressToUse =
SimpleString.toSimpleString(CompositeAddress.extractAddressName(source.getAddress()));
queueNameToUse =
SimpleString.toSimpleString(CompositeAddress.extractQueueName(source.getAddress()));
- } else {
+ } else if (source.getAddress() != null) {
Review comment:
Test?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers
asfgit closed pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers URL: https://github.com/apache/activemq-artemis/pull/2835 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] asfgit merged pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers
asfgit merged pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers URL: https://github.com/apache/activemq-artemis/pull/2835 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] MrEasy opened a new pull request #2836: [ARTEMIS-2487] Updated to org.jctools:jctools-core:2.1.2
MrEasy opened a new pull request #2836: [ARTEMIS-2487] Updated to org.jctools:jctools-core:2.1.2 URL: https://github.com/apache/activemq-artemis/pull/2836 To resolve wrong import range due to wrong declaration in jctools-core:2.1.1 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] jbertram opened a new pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers
jbertram opened a new pull request #2835: ARTEMIS-2489 ring q fails w/concurrent producers URL: https://github.com/apache/activemq-artemis/pull/2835 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [activemq-artemis] lulf opened a new pull request #2834: ARTEMIS-2488: Handle the case where source address is null
lulf opened a new pull request #2834: ARTEMIS-2488: Handle the case where source address is null URL: https://github.com/apache/activemq-artemis/pull/2834 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: us...@infra.apache.org With regards, Apache Git Services
