gemmellr commented on code in PR #4228:
URL: https://github.com/apache/activemq-artemis/pull/4228#discussion_r976438246
##########
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/bin/artemis-service:
##########
@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+ #!/usr/bin/env sh
Review Comment:
Should this be here?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java:
##########
@@ -122,6 +124,9 @@ public void testPropertiesConfigReload() throws Exception {
return mytopic_31.getBindings().size() == 3;
});
+ // verify round trip apply
+
Assert.assertTrue(server.getActiveMQServerControl().getStatus().contains("2"));
Review Comment:
Is the config update application atomic or can updates to one element be
seen mid-application of others? If the latter then this doesnt necessarily seem
to reliably achieve what you want as the 'status' value may be visible before
other changes in the update are actually applied? I guess you could apply a
second status-only change to the config, after the first round is seen, to know
the first update process had finished, since you can see the second update in
effect.
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java:
##########
@@ -521,6 +521,9 @@ public interface ActiveMQServerControl {
@Attribute(desc = "The runtime size of the authorization cache")
long getAuthorizationCacheSize();
+ @Attribute(desc = "The current status of the server")
+ String getStatus();
+
Review Comment:
Having a 'status' attribute ("The current status of the server") which isnt
actually a server status but just a configurable config string that defaults to
empty, seems off to me. I would call this something else either more specific
or more general. Seems more like the intent is to use it as a config update
correlation of sorts, perhaps wind that function into the name?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
