[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=900686&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900686 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 19/Jan/24 11:40 Start Date: 19/Jan/24 11:40 Worklog Time Spent: 10m Work Description: gemmellr merged PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729 Issue Time Tracking --- Worklog Id: (was: 900686) Time Spent: 2h 20m (was: 2h 10m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 2h 20m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=900465&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900465 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 18/Jan/24 16:19 Start Date: 18/Jan/24 16:19 Worklog Time Spent: 10m Work Description: gtully commented on PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#issuecomment-1898798243 that looks good to me. it makes sense that the properties applier will auto create default instances for any intermediary that it encounters while trying to apply nested properties. What we may need is a test for the case where w.x.y.z=1 is set and both getX and getY can return null. Issue Time Tracking --- Worklog Id: (was: 900465) Time Spent: 2h 10m (was: 2h) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 2h 10m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=900454&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900454 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 18/Jan/24 15:44 Start Date: 18/Jan/24 15:44 Worklog Time Spent: 10m Work Description: tabish121 commented on PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#issuecomment-1898733016 > LGTM from my minimal understanding of the properties magic. > > @gtully thoughts? I like the idea, although I remain mostly befuddled by the broker properties code. Issue Time Tracking --- Worklog Id: (was: 900454) Time Spent: 2h (was: 1h 50m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=900448&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900448 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 18/Jan/24 15:28 Start Date: 18/Jan/24 15:28 Worklog Time Spent: 10m Work Description: gemmellr commented on PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#issuecomment-1898699450 LGTM from my minimal understanding of the properties magic. @gtully thoughts? Issue Time Tracking --- Worklog Id: (was: 900448) Time Spent: 1h 50m (was: 1h 40m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1h 50m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=900438&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-900438 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 18/Jan/24 14:01 Start Date: 18/Jan/24 14:01 Worklog Time Spent: 10m Work Description: brusdev commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1457483882 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I completely removed the `enabled` attribute. Issue Time Tracking --- Worklog Id: (was: 900438) Time Spent: 1h 40m (was: 1.5h) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899827&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899827 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 16/Jan/24 09:50 Start Date: 16/Jan/24 09:50 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1453163076 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/CacheConfiguration.java: ## @@ -43,4 +52,12 @@ public CacheConfiguration setTimeout(int timeout) { this.timeout = timeout; return this; } + + public CacheConfiguration() { + + } + + public CacheConfiguration(boolean enabled) { + this.enabled = enabled; + } Review Comment: Can we put the constructors at the top given their importance in this case (will also minimise the diff in this case as one was there already) ## artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java: ## @@ -2871,6 +2871,9 @@ private void parseConnectionRouterConfiguration(final Element e, final Configura } private void parseCacheConfiguration(final Element e, final CacheConfiguration cacheConfiguration) throws ClassNotFoundException { + cacheConfiguration.setEnabled(getBoolean(e, "enabled", + cacheConfiguration.isEnabled())); Review Comment: Feels like this would be better defaulting to simply the literal "true" per the xsd as you had it originally? ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/PoolConfiguration.java: ## @@ -118,4 +129,12 @@ public PoolConfiguration setDiscoveryGroupName(String discoveryGroupName) { this.discoveryGroupName = discoveryGroupName; return this; } + + public PoolConfiguration() { + + } + + public PoolConfiguration(boolean enabled) { + this.enabled = enabled; + } Review Comment: Ditto ## artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java: ## @@ -2889,6 +2892,9 @@ private void parsePolicyConfiguration(final Element e, final NamedPropertyConfig } private void parsePoolConfiguration(final Element e, final Configuration config, final PoolConfiguration poolConfiguration) throws Exception { + poolConfiguration.setEnabled(getBoolean(e, "enabled", + poolConfiguration.isEnabled())); Review Comment: ditto Issue Time Tracking --- Worklog Id: (was: 899827) Time Spent: 1.5h (was: 1h 20m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1.5h > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899779&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899779 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 16/Jan/24 07:53 Start Date: 16/Jan/24 07:53 Worklog Time Spent: 10m Work Description: brusdev commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1453038703 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I reverted the default value of the `enabled` bit for cache and pool configurations to true but I kept the xml attribute `enabled` suggested by @tabish121. The 3 different ways of configuring things seem more consistent: - xml: the `connection-router` element has no cache or pool enabled by default but if you create a cache or pool element they are enabled by default (this is important for backward compatibility) - programmatically: the `ConnectionRouterConfiguration` class has no cache or pool enabled by default but if you create a CacheConfiguration or PoolConfiguration class they are enabled by default (this is important for backward compatibility) - broker properties: the `connectionRouters` items have no cache or pool enabled by default but you can set enabled to true to enable them Issue Time Tracking --- Worklog Id: (was: 899779) Time Spent: 1h 20m (was: 1h 10m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899777&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899777 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 16/Jan/24 07:50 Start Date: 16/Jan/24 07:50 Worklog Time Spent: 10m Work Description: brusdev commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1453038703 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I reverted the default value of the `enabled` bit for cache and pool configurations to true but I keep the xml attribute `enabled` suggested by @tabish121. The 3 different ways of configuring things seem more consistent: - xml: the `connection-router` element has no cache or pool enabled by default but if you create a cache or pool element they are enabled by default (this is important for backward compatibility) - programmatically: the `ConnectionRouterConfiguration` class has no cache or pool enabled by default but if you create a CacheConfiguration or PoolConfiguration class they are enabled by default (this is important for backward compatibility) - broker properties: the `connectionRouters` items have no cache or pool enabled by default but you can set enabled to true to enable them Issue Time Tracking --- Worklog Id: (was: 899777) Time Spent: 1h 10m (was: 1h) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899440&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899440 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 12/Jan/24 15:34 Start Date: 12/Jan/24 15:34 Worklog Time Spent: 10m Work Description: tabish121 commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1450611631 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I'm happy enough that it is at least more obvious now that it is more visible that it is consistently inconsistent between the various configuration paths. Other than more documentation surrounding those inconsistencies I'm not sure how we could fix that sadly. Issue Time Tracking --- Worklog Id: (was: 899440) Time Spent: 1h (was: 50m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899352&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899352 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 12/Jan/24 10:40 Start Date: 12/Jan/24 10:40 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1450218942 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I had the same reaction as Tim to the original code but hadnt got to thinking of an improvement. In some ways this is definitely better, the code is a little more consistent with itself and other previous code. In others its much the same as before; the 'enabled' bit can now be controlled in both configs rather than just one, but there is still effectively 3 different/conflicting 'default' behaviours for it between XML and broker-properties and both, which is confusing. But I dont necessarily have an idea thats nicer to resolve that while there are actually 3 different ways of configuring things. Issue Time Tracking --- Worklog Id: (was: 899352) Time Spent: 50m (was: 40m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899331&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899331 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 12/Jan/24 07:28 Start Date: 12/Jan/24 07:28 Worklog Time Spent: 10m Work Description: brusdev commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1449968857 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: @tabish121 I like your solution, it is more consistent. I changed the implementation. Issue Time Tracking --- Worklog Id: (was: 899331) Time Spent: 40m (was: 0.5h) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899288&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899288 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 11/Jan/24 20:06 Start Date: 11/Jan/24 20:06 Worklog Time Spent: 10m Work Description: tabish121 commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1449339897 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I found this bit particularly confusing as it defaults the value in the constituent configuration objects to true but then forces them to false here. My assumption is that this is somehow tied to trying to make this work both in the XML configuration and the broker properties bits but I'm not 100% sure its consistent between the two. There doesn't appear to be an enable option in the XML but instead seems to imply that the presence alone is enabling but the broker properties path would require the option to be set true even if any of the other configuration is present. In general I'd prefer if the code used a simpler default to false approach and the configuration options were consistent or could be changed such that the user gets a default enabled from XML via the file parser using a default and / or making it configurable in XML. Then it would be nice also doing something consistent in the broker properties. Just looking at the code changes to this point set of red flags for me and I had to chase down why it did this. Issue Time Tracking --- Worklog Id: (was: 899288) Time Spent: 0.5h (was: 20m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=899287&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-899287 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 11/Jan/24 20:05 Start Date: 11/Jan/24 20:05 Worklog Time Spent: 10m Work Description: tabish121 commented on code in PR #4729: URL: https://github.com/apache/activemq-artemis/pull/4729#discussion_r1449339897 ## artemis-server/src/main/java/org/apache/activemq/artemis/core/config/routing/ConnectionRouterConfiguration.java: ## @@ -26,8 +26,8 @@ public class ConnectionRouterConfiguration implements Serializable { private KeyType keyType = KeyType.SOURCE_IP; private String keyFilter = null; private String localTargetFilter = null; - private CacheConfiguration cacheConfiguration = null; - private PoolConfiguration poolConfiguration = null; + private CacheConfiguration cacheConfiguration = new CacheConfiguration().setEnabled(false); + private PoolConfiguration poolConfiguration = new PoolConfiguration().setEnabled(false); Review Comment: I found this bit particularly confusing as it defaults the value in the constituent configuration objects to true but then forces them to false here. My assumption is that this is somehow tied to trying to make this work both in the XML configuration and the broker properties bits but I'm not 100% its consistent between the two. There doesn't appear to be an enable option in the XML but instead seems to imply that the presence alone is enabling but the properties would require the option to be set true even if any of the other configuration is present. In general I'd prefer if the code used a simpler default to false approach and the configuration options where consistent or could be changed such that the user gets a default enabled from XML via the file parser using a default and / or making it configurable in XML. Then it would be nice also doing something consistent in the broker properties. Just looking at the code changes to this point set of red flags for me and I had to chase down why it did this. Issue Time Tracking --- Worklog Id: (was: 899287) Time Spent: 20m (was: 10m) > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4552) Configure all connection-router settings by using broker properties
[ https://issues.apache.org/jira/browse/ARTEMIS-4552?focusedWorklogId=898050&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-898050 ] ASF GitHub Bot logged work on ARTEMIS-4552: --- Author: ASF GitHub Bot Created on: 04/Jan/24 14:32 Start Date: 04/Jan/24 14:32 Worklog Time Spent: 10m Work Description: brusdev opened a new pull request, #4729: URL: https://github.com/apache/activemq-artemis/pull/4729 (no comment) Issue Time Tracking --- Worklog Id: (was: 898050) Remaining Estimate: 0h Time Spent: 10m > Configure all connection-router settings by using broker properties > --- > > Key: ARTEMIS-4552 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4552 > Project: ActiveMQ Artemis > Issue Type: Improvement >Reporter: Domenico Francesco Bruscino >Assignee: Domenico Francesco Bruscino >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > Allow to configure all the connection-router settings by using the broker > properties. The broker properties for the cache configuration cause a null > pointer exception: > {code} > "connectionRouters.autoShard.cacheConfiguration.persisted=true" > "connectionRouters.autoShard.cacheConfiguration.timeout=6" > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)