You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "ehsavoie (via GitHub)" <gi...@apache.org> on 2023/04/11 14:13:38 UTC
[GitHub] [activemq-artemis] ehsavoie opened a new pull request, #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
ehsavoie opened a new pull request, #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433
…nger working.
* Re-adding the missing code
Jira: https://issues.apache.org/jira/browse/ARTEMIS-4238
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433#issuecomment-1503847900
It would be ideal to have some tests for this to catch future regressions.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433#issuecomment-1507083552
@ehsavoie, this looks good. I transitioned it from draft and will merge shortly.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on a diff in pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433#discussion_r1163154561
##########
artemis-ra/src/main/java/org/apache/activemq/artemis/ra/ActiveMQRAManagedConnection.java:
##########
@@ -336,7 +336,7 @@ public void associateConnection(final Object obj) throws ResourceException {
public void checkTransactionActive() throws JMSException {
// don't bother looking at the transaction if there's an active XID
- if (!inManagedTx && tsr != null) {
+ if (!inManagedTx && tsr != null && tsr.getTransactionKey() != null) {
Review Comment:
This change doesn't appear related to the `transactionTimeout` change. Why is this necessary?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram merged pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram merged PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433#issuecomment-1503849936
The commit message doesn't follow the [recommended format](https://github.com/apache/activemq-artemis/blob/main/docs/hacking-guide/en/code.md#commitMessageDetails). There's no need for square brackets and a colon around the Jira reference.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on a diff in pull request #4433: [ARTEMIS-4238]: transaction-timeout ActivationConfigProperty is no lo…
Posted by "ehsavoie (via GitHub)" <gi...@apache.org>.
ehsavoie commented on code in PR #4433:
URL: https://github.com/apache/activemq-artemis/pull/4433#discussion_r1163199931
##########
artemis-ra/src/main/java/org/apache/activemq/artemis/ra/ActiveMQRAManagedConnection.java:
##########
@@ -336,7 +336,7 @@ public void associateConnection(final Object obj) throws ResourceException {
public void checkTransactionActive() throws JMSException {
// don't bother looking at the transaction if there's an active XID
- if (!inManagedTx && tsr != null) {
+ if (!inManagedTx && tsr != null && tsr.getTransactionKey() != null) {
Review Comment:
a leftover
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org