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/06/09 16:47:00 UTC
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull request #3172: ARTEMIS-2797 - Allow for removing current queue filter
michaelandrepearce commented on a change in pull request #3172:
URL: https://github.com/apache/activemq-artemis/pull/3172#discussion_r437564056
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -689,7 +689,10 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration) throws Ex
queue.setDelayBeforeDispatch(queueConfiguration.getDelayBeforeDispatch().longValue());
}
Filter filter = FilterImpl.createFilter(queueConfiguration.getFilterString());
- if (filter != null && !filter.equals(queue.getFilter())) {
+ if ((filter == null) && (queue.getFilter() != null)) {
Review comment:
Should think about applying the same to all, e.g. if now null set back to default for address setting (or static default)
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -689,7 +689,10 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration) throws Ex
queue.setDelayBeforeDispatch(queueConfiguration.getDelayBeforeDispatch().longValue());
}
Filter filter = FilterImpl.createFilter(queueConfiguration.getFilterString());
- if (filter != null && !filter.equals(queue.getFilter())) {
+ if ((filter == null) && (queue.getFilter() != null)) {
Review comment:
Should think about applying the same to all, e.g. if now null set back to default for address setting (or static default) else some users will get different behaviour of what update does per queue config. We should have uniform behaviour across all. E.g. if support here should have similar logic ob all. And also def requires doc updates
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
##########
@@ -689,7 +689,10 @@ public QueueBinding updateQueue(QueueConfiguration queueConfiguration) throws Ex
queue.setDelayBeforeDispatch(queueConfiguration.getDelayBeforeDispatch().longValue());
}
Filter filter = FilterImpl.createFilter(queueConfiguration.getFilterString());
- if (filter != null && !filter.equals(queue.getFilter())) {
+ if ((filter == null) && (queue.getFilter() != null)) {
Review comment:
Should think about applying the same to all, e.g. if now null set back to default for address setting (or static default) else some users will get different behaviour of what update does per queue config. We should have uniform behaviour across all. E.g. if support here should have similar logic ob all. And also def requires doc updates, and a call out in behaviour changes in release notes
----------------------------------------------------------------
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