Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
gemmellr commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852240301
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
I realised the other tests were there, thats why I mentioned them.
I'm not seeing how removing the setExpiryDelay would need a complete rewrite
though, wouldnt it just be a case of adding a setExpiration before sending in
that test, as done by all the other 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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic merged PR #5363: URL: https://github.com/apache/activemq-artemis/pull/5363 -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on PR #5363: URL: https://github.com/apache/activemq-artemis/pull/5363#issuecomment-2491530359 I will merge this... we can do further improvements on following up PRs if needed. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
gemmellr commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r185232
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
Fair enough, you had only mentioned the FQQN one so I thought it was the
only one actually needing this (which is why I didnt like changing it for all
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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852284956
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
other tests are failing in the same situation if I don't set this..
I had added more loops and more sleeps to other tests and they also failed.
I would rather keep this rather to rewrite this test.
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
other tests are failing in the same situation if I don't set this..
I had added more loops and more sleeps to other tests and they also failed.
I would rather keep this rather than rewrite this 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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852292169
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
I will mark it as resolved.. please unmark it if you disagree.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852291226
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
I could also keep running them in loop to add individually on every tests
needed.. so I would rather just keep 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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852284956
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
other tests are failing in the same situation if I don't set this..
I had added more loops and more sleeps to other tests and they also failed.
I would rather keep this instead of rewrite this 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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
gemmellr commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1851947858
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
I'm actually curious why it is setting expiry delay on the regular address
settings to begin with, and whether that call should go away instead of adding
this?
Most of the tests look to send messages that have expiration and are
already-expired, so the setting shouldnt affect them after this change.
The exceptions seem to be testAutoCreatedExpiryFilterAnycast /
testAutoCreatedExpiryFilterMulticast that seem to send messages without
expiration. Could it just change to setting one, and then remove the
ExpiryDelay setting here?
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852215825
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -129,7 +130,7 @@ public void testAutoCreatedExpiryFilterMulticast() throws
Exception {
}
private void testAutoCreatedExpiryFilter(RoutingType routingType) throws
Exception {
- final int ITERATIONS = 50;
+ final int ITERATIONS = 5;
Review Comment:
no need for 50 iterations.. I'm lowering it down to save a few seconds.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852170707
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -194,6 +195,8 @@ public void testWithJMSFQQN() throws Exception {
triggerExpiration();
+ Thread.sleep(EXPIRY_DELAY);
+
Review Comment:
I actually added a comment why
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852123640
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
There are other tests in this test class that will use this:
https://github.com/apache/activemq-artemis/blob/14c564a48144b0e052c6e064ba0f94744335/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java#L136-L138
I will use a prefix rather than everything.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1852142815
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
It would be a complete rewrite of the test.. I will just go with the extra
line.
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on PR #5363: URL: https://github.com/apache/activemq-artemis/pull/5363#issuecomment-2489998328 It's currently showing this, when null: null@tcp://localhost:61616 > -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363: URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1851997793 ## docs/user-manual/message-expiry.adoc: ## @@ -76,6 +76,14 @@ If `min-expiry-delay` is not defined then the message will not be changed. The default for both `min-expiry-delay` and `max-expiry-delay` is `-1` (i.e. disabled). +[IMPORTANT] +.Warning + +**If you set expiry-delay on the target expiration target address beware of the following:** + +* Messages will get a new expiration when moved, and may disappear after the new expiration. + Review Comment: this was my very first asciidoc :) thanks! I had the impression I needed to do it that way (the WARNING thing) -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1851995014
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -194,6 +195,8 @@ public void testWithJMSFQQN() throws Exception {
triggerExpiration();
+ Thread.sleep(EXPIRY_DELAY);
+
Review Comment:
this was to make sure the message was not expiring... I needed that to
reproduce the failure it was failing more consistently.
However I think I can take it out now that I ruled it out by 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.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
gemmellr commented on code in PR #5363:
URL: https://github.com/apache/activemq-artemis/pull/5363#discussion_r1851948116
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -194,6 +195,8 @@ public void testWithJMSFQQN() throws Exception {
triggerExpiration();
+ Thread.sleep(EXPIRY_DELAY);
+
Review Comment:
Is this actually needed? If so maybe a comment why given the other tests
dont do it? Given the triggerExpiration method before seems to send an
already-expired message, and the later receive here already has a timeout.
##
docs/user-manual/message-expiry.adoc:
##
@@ -76,6 +76,14 @@ If `min-expiry-delay` is not defined then the message will
not be changed.
The default for both `min-expiry-delay` and `max-expiry-delay` is `-1` (i.e.
disabled).
+[IMPORTANT]
+.Warning
+
+**If you set expiry-delay on the target expiration target address beware of
the following:**
+
+* Messages will get a new expiration when moved, and may disappear after the
new expiration.
+
Review Comment:
Seems weird to use the IMPORTANT admonition and then start it with a
'Warning heading'. If its a warning, why not just use the WARNING admonition? I
dont think we need the additional text heading at all, it just distracts from
getting the actual message across, the shouty icon and bold sentence seem more
than enough implication already.
The same thing will happen with min-expiry-delay/max-expiry-delay so they
should be mentioned too, especially as this right is after their documentation.
It could also clarify that the expiration is normally set to 0 when moving
to the expiry queue.
```suggestion
[WARNING]
**If you set expiry-delay, or min/max-expiry-delay, on the expiration target
address beware of the following:**
* Messages will get a new expiration when moved to the expiry queue, rather
than being set to 0 as usual, and so may disappear after the new expiration.
```
##
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/AutoCreateExpiryResourcesTest.java:
##
@@ -59,6 +59,7 @@ public void setUp() throws Exception {
// set common address settings needed for all tests; make sure to use
getMatch instead of addMatch in invidual tests or these will be overwritten
server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setAutoCreateExpiryResources(true).setExpiryAddress(expiryAddress).setExpiryDelay(EXPIRY_DELAY));
+
server.getAddressSettingsRepository().addMatch(String.valueOf(expiryAddress),
new AddressSettings().setExpiryDelay(-1L));
Review Comment:
I'm actually curious why it is setting expiry delay on the regular address
settings to begin with, and whether that should instead of adding this?
Most of the tests look to send messages that have expiration and are
already-expired, so the setting shouldnt affect them after this change.
The exceptions seem to be testAutoCreatedExpiryFilterAnycast /
testAutoCreatedExpiryFilterMulticast that seem to send messages without
expiration. Could it just change to setting one, and then remove the
ExpiryDelay setting here?
--
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-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5170 Fix AutoCreateExpiryResourcesTest and document expiry of expiry [activemq-artemis]
clebertsuconic commented on PR #5363: URL: https://github.com/apache/activemq-artemis/pull/5363#issuecomment-2489998018 I really liked this... can I ask a small change.. if you can't do it.. I will do it later... if the user is null.. just don't set the user If I have a broker without an user (anonymous user).. it's currently showing null@tcp://localhost:61616 > I think it should use just tcp://localhost:61616 > although the check that you have on the Exception would need to be different.. perhaps just use a boolean to connected instead. -- 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-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact
