You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "clebertsuconic (via GitHub)" <gi...@apache.org> on 2023/07/24 17:42:13 UTC
[GitHub] [activemq-artemis] clebertsuconic opened a new pull request, #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
clebertsuconic opened a new pull request, #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565
(no comment)
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276559089
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java:
##########
@@ -36,11 +34,12 @@
import org.apache.activemq.artemis.nativo.jlibaio.LibaioContext;
import org.apache.activemq.artemis.nativo.jlibaio.LibaioFile;
import org.apache.activemq.artemis.utils.FileUtil;
+import picocli.CommandLine;
/**
* CLI action that creates a broker instance directory.
*/
-@Command(name = "create", description = "Create a new broker instance.")
+@CommandLine.Command(name = "create", description = "Create a new broker instance.")
Review Comment:
Dont think it should need to be static, its just a nested interface/class, regular import of the full name should work.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277446167
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -159,11 +176,19 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
try {
Connection connection = cf.createConnection();
connection.close();
+ connectionSucces(brokerURL, user, password);
Review Comment:
the connection is already called on line 177
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277680335
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/DestAbstract.java:
##########
@@ -21,29 +21,29 @@
import javax.jms.JMSException;
import javax.jms.Session;
-import com.github.rvesse.airline.annotations.Option;
import org.apache.activemq.artemis.cli.factory.serialize.MessageSerializer;
import org.apache.activemq.artemis.cli.factory.serialize.XMLMessageSerializer;
import org.apache.activemq.artemis.jms.client.ActiveMQDestination;
+import picocli.CommandLine.Option;
public class DestAbstract extends ConnectionAbstract {
- @Option(name = "--destination", description = "Destination to be used. It can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
+ @Option(names = "--destination", description = "Destination to be used. It can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
String destination = "queue://TEST";
- @Option(name = "--message-count", description = "Number of messages to act on. Default: 1000.")
+ @Option(names = "--message-count", description = "Number of messages to act on. Default: 1000.")
int messageCount = 1000;
- @Option(name = "--sleep", description = "Time wait between each message.")
+ @Option(names = "--sleep", description = "Time wait between each message.")
int sleep = 0;
- @Option(name = "--txt-size", description = "Transaction batch size.")
+ @Option(names = {"--txt-size", "--tx-batch-size"}, description = "Transaction batch size. (--txt-size is deprecated)")
Review Comment:
I'm using hidden now...
and I opened a PR and bug on pico to fix the auto-completion on the shell:
https://github.com/remkop/picocli/pull/2075
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276568011
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/UserGroup.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli.commands.user;
+
+import org.apache.activemq.artemis.cli.commands.HelpAction;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "user", description = "file-based user management (example ./artemis user list)", subcommands = {ListUser.class, AddUser.class, RemoveUser.class, ResetUser.class})
Review Comment:
These group examples like "(example ./artemis user list)" show up in the shell help output, where it is actually _not_ what should be done. Not sure if it could cater to shell vs not-shell usage mode, but might be nice if it was possible.
--
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 #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1272579649
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/AutoCompletion.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli.commands;
+
+import java.io.File;
+
+import picocli.AutoComplete;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "auto-complete", description = "Generates the auto complete helper file")
+public class AutoCompletion implements Runnable {
+
+ public AutoCompletion(CommandLine parent) {
+ this.parent = parent;
+ }
+
+ CommandLine parent;
+
+ @CommandLine.Parameters (description = "The script that will be used to invoke check-leak")
Review Comment:
Reference to "check-leak" here seems wrong.
--
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] clebertsuconic merged pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276538252
##########
docs/user-manual/en/using-server.md:
##########
@@ -50,6 +50,30 @@ The following highlights some important folders on the distribution:
- `user-manual` - The user manual is placed under the web folder.
+## Artemis Shell
+
+You can activate the artemis shell by typing:
+
+```shell
+./artemis shell
+```
+
+This will start a JLine3 terminal with auto-completion:
Review Comment:
A note on how to exit the shell may also be in order, some people wont actually know.
--
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] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1654801169
I will finish the other 2 items tomorrow. there's one item I didn't close (resolve) from our conversation here because I'm not sure it's the best choice yet.. will look at it again tomorrow.
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277379207
##########
docs/user-manual/en/using-server.md:
##########
@@ -50,6 +50,30 @@ The following highlights some important folders on the distribution:
- `user-manual` - The user manual is placed under the web folder.
+## Artemis Shell
+
+You can activate the artemis shell by typing:
+
+```shell
+./artemis shell
+```
+
+This will start a JLine3 terminal with auto-completion:
Review Comment:
unresolved as the docs didnt seem to change
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276528603
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java:
##########
@@ -36,11 +34,12 @@
import org.apache.activemq.artemis.nativo.jlibaio.LibaioContext;
import org.apache.activemq.artemis.nativo.jlibaio.LibaioFile;
import org.apache.activemq.artemis.utils.FileUtil;
+import picocli.CommandLine;
/**
* CLI action that creates a broker instance directory.
*/
-@Command(name = "create", description = "Create a new broker instance.")
+@CommandLine.Command(name = "create", description = "Create a new broker instance.")
Review Comment:
we could use import static.. which I don't like much.
But I can make an exception 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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277341990
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -172,6 +197,13 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
// if a connection exception will ask for the URL, user and password
getActionContext().err.println("Connection failed::" + e.getMessage());
cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), inputBrokerURL(brokerURL));
+ try {
+ Connection connection = cf.createConnection();
+ connection.close();
+ connectionSucces(brokerURL, user, password);
Review Comment:
Use tryConnect() like done elsewhere?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/DestAbstract.java:
##########
@@ -21,29 +21,29 @@
import javax.jms.JMSException;
import javax.jms.Session;
-import com.github.rvesse.airline.annotations.Option;
import org.apache.activemq.artemis.cli.factory.serialize.MessageSerializer;
import org.apache.activemq.artemis.cli.factory.serialize.XMLMessageSerializer;
import org.apache.activemq.artemis.jms.client.ActiveMQDestination;
+import picocli.CommandLine.Option;
public class DestAbstract extends ConnectionAbstract {
- @Option(name = "--destination", description = "Destination to be used. It can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
+ @Option(names = "--destination", description = "Destination to be used. It can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
String destination = "queue://TEST";
- @Option(name = "--message-count", description = "Number of messages to act on. Default: 1000.")
+ @Option(names = "--message-count", description = "Number of messages to act on. Default: 1000.")
int messageCount = 1000;
- @Option(name = "--sleep", description = "Time wait between each message.")
+ @Option(names = "--sleep", description = "Time wait between each message.")
int sleep = 0;
- @Option(name = "--txt-size", description = "Transaction batch size.")
+ @Option(names = {"--txt-size", "--tx-batch-size"}, description = "Transaction batch size. (--txt-size is deprecated)")
Review Comment:
The non-deprecated option name being first would read better (unless the cli is reverse-ordering the options)
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,62 +189,111 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+ if (userObject instanceof Action) {
Review Comment:
Maybe add an Objects.requireNonNull() here...or otherwise cover potential for NPE when throwing the new IAE below.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -180,28 +212,62 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverSuccessPassword();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverSuccessPassword() {
+ if (CONNECTION_SUCCESS.get() != null) {
+ ConnectionSuccess success = CONNECTION_SUCCESS.get();
+ if (this.user == null) {
+ this.user = success.user;
+ }
+ if (this.password == null) {
+ this.password = success.password;
+ }
+ if (this.brokerURL == null) {
+ this.brokerURL = success.uri;
+ }
+
+ }
+ }
+
+ void connectionSucces(String brokerURL, String user, String password) {
Review Comment:
Typo in method name, missing s at end...connectionSucceeded might be better.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java:
##########
@@ -205,6 +205,20 @@ the ARTEMIS_HOME variable will include back slashes (An invalid file URI charact
return brokerHome;
}
+ @Override
+ public void run() {
+ try {
+ execute(getActionContext());
+ } catch (Throwable e) {
+ e.printStackTrace();
Review Comment:
Is this catering to the shell usage, which already catches+prints by itself? Or regular CLI usage?
Actually..when is this run() implementation used exactly? The 'internal execute' method is specifically checking for Action and handling it first...so wont all of the stuff using this abstract class go through that path instead?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -159,11 +176,19 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
try {
Connection connection = cf.createConnection();
connection.close();
+ connectionSucces(brokerURL, user, password);
Review Comment:
Use tryConnect() like done elsewhere?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -180,28 +212,62 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverSuccessPassword();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverSuccessPassword() {
Review Comment:
Its doing more than the name suggests. A clearer name, maybe also around the expected usage, would be better. Perhaps recoverSavedConnectionInfo? Similarly renaming the class/fields below something related around 'connection info'.
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1280291519
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,68 +191,140 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+
+ // Pico shouldn't allow generating a commandLine without an userObject.
+ // the following assert "should" never happen
+ assert userObject != null;
Review Comment:
Fair enough. You didnt note the last bit before, just that 'we shouldnt add things that arent Runnable or Action', then you added the fallback check which would NPE if it was null, which I then noted. I'd still have just added a simple Objects.nonNull("oh-dear developer-error", userObject) rather than the multi line assert+comment that quite posibly still wont do anything and allow the raw NPE to occur.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277673400
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -87,9 +68,29 @@
* Notice that this class should not use any logging as it's part of the bootstrap and using logging here could
* disrupt the order of bootstrapping on certain components (e.g. JMX being started from log4j)
*/
-public class Artemis {
+@Command(name = "artemis", description = "ActiveMQ Artemis Command Line")
+public class Artemis implements Runnable {
+
+ CommandLine commandLine;
+
+ public CommandLine getCommandLine() {
+ return commandLine;
+ }
+
+ public Artemis setCommandLine(CommandLine commandLine) {
+ this.commandLine = commandLine;
+ return this;
+ }
+
+ @Override
+ public void run() {
+ // We are running the shell by default.
+ // if you type ./artemis we will go straight to the shell
+ Shell.runShell();
+ }
public static void main(String... args) throws Exception {
+ System.out.println("Line::" + getNameFromBanner());
Review Comment:
yeah!! lol thank you
--
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] gemmellr commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1655465910
> I have added one more review from myself. When using user/password the system would ask me for the user/password all the time. I'm now saving the last successful user/password between executions, adding a connect and disconnect that's only available through the shell (what makes the shell more useful since it won't ask for user/password every time when using security)
I think the ability to set and clear the URI + credentials used for subsequent commands in the shell is good. I'm less clear on it doing it silently in the background based on every previous command.
At least one of the additions you made was around it handling the URIs crossing between the different clients, presumably as the URI is saved but the protocol value is not, probably how you hit that, the client used will change between commands in some cases, whilst the URI + credentials etc do not even though the URIs are client-specific.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279627523
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,68 +191,140 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+
+ // Pico shouldn't allow generating a commandLine without an userObject.
+ // the following assert "should" never happen
+ assert userObject != null;
Review Comment:
this is never supposed to happen.
Only chance userObject is null is if you set the userObject wrongly during development. and picocli actually validate that already.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279267330
##########
artemis-cli/pom.xml:
##########
@@ -124,10 +124,17 @@
<artifactId>jakarta.xml.bind-api</artifactId>
<version>${jakarta.xml.bind-api.version}</version>
</dependency>
-
<dependency>
- <groupId>com.github.rvesse</groupId>
- <artifactId>airline</artifactId>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli-shell-jline3</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.jline</groupId>
+ <artifactId>jline</artifactId>
Review Comment:
@gnodet thank you so much for your help 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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277452609
##########
docs/user-manual/en/using-server.md:
##########
@@ -50,6 +50,30 @@ The following highlights some important folders on the distribution:
- `user-manual` - The user manual is placed under the web folder.
+## Artemis Shell
+
+You can activate the artemis shell by typing:
+
+```shell
+./artemis shell
+```
+
+This will start a JLine3 terminal with auto-completion:
Review Comment:
I removed the line.. and I'm adding a chapter for the CLI.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276523807
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,62 +186,102 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+ if (userObject instanceof Action) {
+ Action action = (Action) userObject;
+ action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ if (action.isVerbose()) {
+ context.out.print("Executing " + action.getClass().getName() + " ");
+ for (String arg : args) {
+ context.out.print(arg + " ");
+ }
+ context.out.println();
+ context.out.println("Home::" + action.getBrokerHome() + ", Instance::" + action.getBrokerInstance());
+ }
- if (action.isVerbose()) {
- context.out.print("Executing " + action.getClass().getName() + " ");
- for (String arg : args) {
- context.out.print(arg + " ");
+ return action.execute(context);
+ } else {
+ if (userObject instanceof Runnable) {
+ ((Runnable) userObject).run();
+ }
Review Comment:
we shouldn't really add anything to do the CommandLine that's not a Runnable or an Action. But I will add a check.
That should prevent future devs from making a mistake.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277446167
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -159,11 +176,19 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
try {
Connection connection = cf.createConnection();
connection.close();
+ connectionSucces(brokerURL, user, password);
Review Comment:
the connection is already called on line 177
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276787067
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/UserGroup.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli.commands.user;
+
+import org.apache.activemq.artemis.cli.commands.HelpAction;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "user", description = "file-based user management (example ./artemis user list)", subcommands = {ListUser.class, AddUser.class, RemoveUser.class, ResetUser.class})
Review Comment:
My options here are limited. I could create a different DataGroup for ShellDataGroup, or I can make a more generic text.
The other option would be some horrible hacking into the internal API.. which would be even worse. So I will go for the more generic text.
--
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] gnodet commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279185868
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Shell.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
+
+import org.jline.console.SystemRegistry;
+import org.jline.console.impl.SystemRegistryImpl;
+import org.jline.reader.EndOfFileException;
+import org.jline.reader.LineReader;
+import org.jline.reader.LineReaderBuilder;
+import org.jline.reader.MaskingCallback;
+import org.jline.reader.Parser;
+import org.jline.reader.UserInterruptException;
+import org.jline.reader.impl.DefaultParser;
+import org.jline.terminal.Terminal;
+import org.jline.terminal.TerminalBuilder;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.shell.jline3.PicocliCommands;
+
+@Command(name = "shell", description = "JLine3 shell helping using the CLI")
+public class Shell implements Runnable {
+
+ private static String RED_UNICODE = "\u001B[31m";
+ private static String YELLOW_UNICODE = "\u001B[33m";
+ private static String CLEAR_UNICODE = "\u001B[0m";
+
+ public Shell(CommandLine commandLine) {
+ }
+
+ @Override
+ public void run() {
+ runShell();
+ }
+
+ private static ThreadLocal<AtomicBoolean> IN_SHELL = ThreadLocal.withInitial(() -> new AtomicBoolean(false));
+
+ public static boolean inShell() {
+ return IN_SHELL.get().get();
+ }
+
+ public static void runShell() {
+ try {
+ IN_SHELL.get().set(true);
+
+ boolean isInstance = System.getProperty("artemis.instance") != null;
+
+ Supplier<Path> workDir = () -> Paths.get(System.getProperty("user.dir"));
+
+ PicocliCommands.PicocliCommandsFactory factory = new PicocliCommands.PicocliCommandsFactory();
+
+ CommandLine commandLine = Artemis.buildCommand(isInstance, !isInstance, true);
+
+ PicocliCommands picocliCommands = new PicocliCommands(commandLine);
+
+ Parser parser = new DefaultParser();
+ try (Terminal terminal = TerminalBuilder.builder().build()) {
Review Comment:
As a shortcut, you could use `TerminalBuilder.terminal()`
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
See above, adding jansi will remove the need for those options.
##########
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/bin/artemis:
##########
@@ -102,6 +102,9 @@ fi
exec "$JAVACMD" \
$JAVA_ARGS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
Adding the jansi library will remove the need for those additional JVM options. Those are only required when using the exec provider, which won't be used if you add jansi to support windows.
##########
artemis-cli/pom.xml:
##########
@@ -124,10 +124,17 @@
<artifactId>jakarta.xml.bind-api</artifactId>
<version>${jakarta.xml.bind-api.version}</version>
</dependency>
-
<dependency>
- <groupId>com.github.rvesse</groupId>
- <artifactId>airline</artifactId>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli-shell-jline3</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.jline</groupId>
+ <artifactId>jline</artifactId>
Review Comment:
In order to support Windows properly, you need to add a dependency on jansi or jna. I suggest jansi which is more lightweight.
```
<dependency>
<groupId>org.fusesource.jansi</groupId>
<artifactId>jansi</artifactId>
<version>2.4.0</version>
</dependency>
```
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1280044335
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -157,68 +174,132 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e.printStackTrace();
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), inputBrokerURL(brokerURL));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e2.printStackTrace();
+ }
return cf;
}
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverConnectionInformation();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverConnectionInformation() {
+ if (CONNECTION_INFORMATION.get() != null) {
+ ConnectionInformation connectionInfo = CONNECTION_INFORMATION.get();
+ if (this.user == null) {
+ this.user = connectionInfo.user;
+ }
+ if (this.password == null) {
+ this.password = connectionInfo.password;
+ }
+ if (this.brokerURL == null) {
+ this.brokerURL = connectionInfo.uri;
+ }
+
+ }
+ }
+
+ void saveConnectionInfo(String brokerURL, String user, String password) {
+ if (Shell.inShell() && CONNECTION_INFORMATION.get() == null) {
+ CONNECTION_INFORMATION.set(new ConnectionInformation(brokerURL, user, password));
+ System.out.println("CLI connected to broker " + brokerURL + ", user:" + user);
+ }
+ }
+
protected ActiveMQConnectionFactory createCoreConnectionFactory(String brokerURL,
String user,
String password,
String clientID) {
+ if (brokerURL.startsWith("amqp://")) {
+ // replacing amqp:// by tcp://
+ brokerURL = "tcp" + brokerURL.substring(4);
+ }
+
ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
getActionContext().out.println("Consumer:: clientID = " + clientID);
cf.setClientID(clientID);
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(brokerURL, inputUser(user), inputPassword(password));
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(inputBrokerURL(brokerURL), inputUser(user), inputPassword(password));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
Review Comment:
I see what you mean now.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279258496
##########
artemis-cli/pom.xml:
##########
@@ -124,10 +124,17 @@
<artifactId>jakarta.xml.bind-api</artifactId>
<version>${jakarta.xml.bind-api.version}</version>
</dependency>
-
<dependency>
- <groupId>com.github.rvesse</groupId>
- <artifactId>airline</artifactId>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli-shell-jline3</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.jline</groupId>
+ <artifactId>jline</artifactId>
Review Comment:
@gnodet thank you so much.. that worked like a charm!
@gemmellr ^^^ C/C
--
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] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1653444177
https://github.com/apache/activemq-artemis/assets/750514/2e094597-8172-44e6-9965-e88766c396e2
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276957650
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/UserGroup.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli.commands.user;
+
+import org.apache.activemq.artemis.cli.commands.HelpAction;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "user", description = "file-based user management (example ./artemis user list)", subcommands = {ListUser.class, AddUser.class, RemoveUser.class, ResetUser.class})
Review Comment:
I think I have solved this..
@gemmellr but I will let you decide if this one could be resolved or not
--
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] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1655863616
for my future reference. I opened an issue and Pull Request on Pico: https://github.com/remkop/picocli/pull/2075
the jshell auto completion is showing hidden values. the PR sent would fix it but I'm waiting for a best way to merge this upstream in Pico. hidden is the way to go to "deprecate" stuff.
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1278982761
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
Its not actually surprising that behaviour here might differ on different JDKs, thats exactly why I wondered if you tried it on the newer ones, especially with JDK21 almost done (RC1 due in 10 days, later RC2 if needed; JDK17 didnt get an RC2)...though I was more concerned it may need other config, than not need any, we will need to use what works for them all.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277677177
##########
artemis-distribution/src/main/resources/licenses/bin/LICENSE:
##########
@@ -286,6 +286,12 @@ For HdrHistogram:
This product bundles HdrHistogram, which is available under the
"2-clause BSD" license. For details, see licences/LICENSE-hdrhistogram.txt.
+==============================================================================
+For JLine:
+==============================================================================
+This product bundles JLine, which is available under the
+"3-clause BSD" license. For details, see licences/LICENSE-jline.txt.
Review Comment:
It was added here but apparently .txt is on the gitignore and I missed adding it. thank you.
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279379655
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -157,68 +174,132 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e.printStackTrace();
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), inputBrokerURL(brokerURL));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e2.printStackTrace();
+ }
return cf;
}
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverConnectionInformation();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverConnectionInformation() {
+ if (CONNECTION_INFORMATION.get() != null) {
+ ConnectionInformation connectionInfo = CONNECTION_INFORMATION.get();
+ if (this.user == null) {
+ this.user = connectionInfo.user;
+ }
+ if (this.password == null) {
+ this.password = connectionInfo.password;
+ }
+ if (this.brokerURL == null) {
+ this.brokerURL = connectionInfo.uri;
+ }
+
+ }
+ }
+
+ void saveConnectionInfo(String brokerURL, String user, String password) {
+ if (Shell.inShell() && CONNECTION_INFORMATION.get() == null) {
+ CONNECTION_INFORMATION.set(new ConnectionInformation(brokerURL, user, password));
+ System.out.println("CLI connected to broker " + brokerURL + ", user:" + user);
+ }
+ }
+
protected ActiveMQConnectionFactory createCoreConnectionFactory(String brokerURL,
String user,
String password,
String clientID) {
+ if (brokerURL.startsWith("amqp://")) {
+ // replacing amqp:// by tcp://
+ brokerURL = "tcp" + brokerURL.substring(4);
+ }
+
ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
getActionContext().out.println("Consumer:: clientID = " + clientID);
cf.setClientID(clientID);
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(brokerURL, inputUser(user), inputPassword(password));
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
Review Comment:
This one does nothing, the earlier ones for Qpid JMS print the stack. Seems like they should be the same one way or the other.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -157,68 +174,132 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e.printStackTrace();
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), inputBrokerURL(brokerURL));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e2.printStackTrace();
+ }
return cf;
}
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverConnectionInformation();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverConnectionInformation() {
+ if (CONNECTION_INFORMATION.get() != null) {
+ ConnectionInformation connectionInfo = CONNECTION_INFORMATION.get();
+ if (this.user == null) {
+ this.user = connectionInfo.user;
+ }
+ if (this.password == null) {
+ this.password = connectionInfo.password;
+ }
+ if (this.brokerURL == null) {
+ this.brokerURL = connectionInfo.uri;
+ }
+
+ }
+ }
+
+ void saveConnectionInfo(String brokerURL, String user, String password) {
+ if (Shell.inShell() && CONNECTION_INFORMATION.get() == null) {
+ CONNECTION_INFORMATION.set(new ConnectionInformation(brokerURL, user, password));
+ System.out.println("CLI connected to broker " + brokerURL + ", user:" + user);
+ }
+ }
+
protected ActiveMQConnectionFactory createCoreConnectionFactory(String brokerURL,
String user,
String password,
String clientID) {
+ if (brokerURL.startsWith("amqp://")) {
+ // replacing amqp:// by tcp://
+ brokerURL = "tcp" + brokerURL.substring(4);
+ }
+
ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
getActionContext().out.println("Consumer:: clientID = " + clientID);
cf.setClientID(clientID);
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(brokerURL, inputUser(user), inputPassword(password));
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(inputBrokerURL(brokerURL), inputUser(user), inputPassword(password));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
Review Comment:
Ditto
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/DestAbstract.java:
##########
@@ -137,4 +141,16 @@ public DestAbstract setSerializer(String serializer) {
this.serializer = serializer;
return this;
}
+
+ @Override
+ public Object execute(ActionContext context) throws Exception {
+ super.execute(context);
+
+ if (oldBatchSize > 0) {
+ context.out.println("--txt-size is deprecated, please use --commit-interval");
+ txBatchSize = oldBatchSize;
Review Comment:
If both were somehow set, I would expect the new one to always win...here the old one will win.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,68 +191,140 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+
+ // Pico shouldn't allow generating a commandLine without an userObject.
+ // the following assert "should" never happen
+ assert userObject != null;
Review Comment:
Good chance the assert also wont happen unless people turn on assertions. Seems like a null check would have been simpler. Though it will at least still NPE later, when failing to throw the IAE.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/perf/PerfCommand.java:
##########
@@ -26,46 +26,56 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.LockSupport;
-import com.github.rvesse.airline.annotations.Arguments;
-import com.github.rvesse.airline.annotations.Option;
import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.cli.commands.messages.ConnectionAbstract;
import org.apache.activemq.artemis.cli.commands.messages.DestAbstract;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.Parameters;
import static java.util.Collections.singletonList;
public abstract class PerfCommand extends ConnectionAbstract {
- @Option(name = "--show-latency", description = "Show latencies at interval on output. Default: disabled.")
+ @Option(names = "--show-latency", description = "Show latencies at interval on output. Default: disabled.")
protected boolean showLatency = false;
- @Option(name = "--json", description = "Report file name. Default: none.")
+ @Option(names = "--json", description = "Report file name. Default: none.")
protected String reportFileName = null;
- @Option(name = "--hdr", description = "HDR Histogram Report file name. Default: none.")
+ @Option(names = "--hdr", description = "HDR Histogram Report file name. Default: none.")
protected String hdrFileName = null;
- @Option(name = "--duration", description = "Test duration (in seconds). Default: 0.")
+ @Option(names = "--duration", description = "Test duration (in seconds). Default: 0.")
protected int duration = 0;
- @Option(name = "--warmup", description = "Warmup time (in seconds). Default: 0.")
+ @Option(names = "--warmup", description = "Warmup time (in seconds). Default: 0.")
protected int warmup = 0;
- @Option(name = "--message-count", description = "Total number of messages. Default: 0.")
+ @Option(names = "--message-count", description = "Total number of messages. Default: 0.")
protected long messageCount = 0;
- @Option(name = "--num-destinations", description = "If present, generate --num-destinations for each destination name, using it as a prefix and adding a number [0,--num-destinations) as suffix. Default: none.")
+ @Option(names = "--num-destinations", description = "If present, generate --num-destinations for each destination name, using it as a prefix and adding a number [0,--num-destinations) as suffix. Default: none.")
protected int numDestinations = 1;
- @Arguments(description = "List of destination names. Each name can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
+ @Option(names = "--tx-size", description = "Transaction size.", hidden = true)
+ protected long txSize;
+
+ @Option(names = "--commit-interval", description = "Transaction size.")
+ protected long commitInterval;
+
+ @Parameters(description = "List of destination names. Each name can be prefixed with queue:// or topic:// and can be an FQQN in the form of <address>::<queue>. Default: queue://TEST.")
protected List<String> destinations;
private final CountDownLatch completed = new CountDownLatch(1);
@Override
public Object execute(ActionContext context) throws Exception {
super.execute(context);
- final ConnectionFactory factory = createConnectionFactory(brokerURL, user, password, null, getProtocol());
+ if (txSize > 0) {
+ System.out.println("--tx-size is deprecated, please use --commit-interval");
+ commitInterval = txSize;
+ }
Review Comment:
If both were somehow set, I would expect the new one to always win...here the old one will win.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1278010468
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
hmmmmmmm... I don't need these with JDK17.
@gnodet it would be so nice to get your hep 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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276148633
##########
pom.xml:
##########
@@ -586,17 +587,23 @@
<!-- License: Apache 2.0 -->
</dependency>
<dependency>
- <groupId>com.github.rvesse</groupId>
- <artifactId>airline</artifactId>
- <version>${airline.version}</version>
- <exclusions>
- <exclusion>
- <groupId>com.github.rvesse</groupId>
- <artifactId>airline-backcompat-javaxinject</artifactId>
- </exclusion>
- </exclusions>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli</artifactId>
+ <version>${picocli.version}</version>
<!-- License: Apache 2.0 -->
</dependency>
+ <dependency>
+ <groupId>info.picocli</groupId>
+ <artifactId>picocli-shell-jline3</artifactId>
+ <version>${picocli.version}</version>
+ <!-- License: Apache 2.0 -->
+ </dependency>
+ <dependency>
+ <groupId>org.jline</groupId>
+ <artifactId>jline</artifactId>
+ <version>${jline.version}</version>
+ <!-- License: BSD 3-Clause -->
+ </dependency>
Review Comment:
Should update the binary assembly LICENCE file to cover this addition since it isnt ALv2.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java:
##########
@@ -36,11 +34,12 @@
import org.apache.activemq.artemis.nativo.jlibaio.LibaioContext;
import org.apache.activemq.artemis.nativo.jlibaio.LibaioFile;
import org.apache.activemq.artemis.utils.FileUtil;
+import picocli.CommandLine;
/**
* CLI action that creates a broker instance directory.
*/
-@Command(name = "create", description = "Create a new broker instance.")
+@CommandLine.Command(name = "create", description = "Create a new broker instance.")
Review Comment:
Do these actually need to be prefixed? Wouldnt it just work the same as before if you simply switched the imports and left the types unchanged where the name is the same? If so the lines would then be shorter/nicer and the diff would be much simpler (just adding the s to make 'name' become 'names').
Same for most of the other files.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,62 +186,102 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+ if (userObject instanceof Action) {
+ Action action = (Action) userObject;
+ action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ if (action.isVerbose()) {
+ context.out.print("Executing " + action.getClass().getName() + " ");
+ for (String arg : args) {
+ context.out.print(arg + " ");
+ }
+ context.out.println();
+ context.out.println("Home::" + action.getBrokerHome() + ", Instance::" + action.getBrokerInstance());
+ }
- if (action.isVerbose()) {
- context.out.print("Executing " + action.getClass().getName() + " ");
- for (String arg : args) {
- context.out.print(arg + " ");
+ return action.execute(context);
+ } else {
+ if (userObject instanceof Runnable) {
+ ((Runnable) userObject).run();
+ }
+ }
+ return null;
+ }
+
+ /*
+ Pico-cli traditionally would execute user objects that implement Runnable.
+ However as we used airline before, we needed parse for the proper action.
+ This method here is parsing the arg and find the proper user object in the hierarchy of sub-commands
+ and return it to the caller.
+ */
+ private static Object parseAction(CommandLine line, String[] args) {
+ CommandLine.ParseResult parseResult = line.parseArgs(args);
+ if (parseResult != null) {
+ for (; parseResult.hasSubcommand(); parseResult = parseResult.subcommand()) {
Review Comment:
While instead of for?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java:
##########
@@ -106,8 +106,16 @@ public Object run(ActionContext context) throws Exception {
throw new IOException(etcFolder + " does not exist for etc");
}
+ StringBuffer javaOption = new StringBuffer();
Review Comment:
StringBuilder?
Name could be more obvious than the singular of the original plural list, e.g javaOptionsArgs...or javaOptionsString (and build the value here rather than later at the point it is passed).
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
Ick. Maybe add a comment to say it is for the CLI stuff.
Have you tried it on JDK17 and 21EA to check nothing different is needed there than on 11?
##########
docs/user-manual/en/using-server.md:
##########
@@ -50,6 +50,30 @@ The following highlights some important folders on the distribution:
- `user-manual` - The user manual is placed under the web folder.
+## Artemis Shell
+
+You can activate the artemis shell by typing:
+
+```shell
+./artemis shell
+```
+
+This will start a JLine3 terminal with auto-completion:
Review Comment:
Sentence fragment, seems to end mid:
---
Also, this drop-in about the shell usage also doesnt really fit with the rest of the existing information on the page below that all assumes you are **not** using it. It could do with some elaboration / description linking and contrasting the different usages so the two routes are clear, and some new user actually following this page linearly isnt met with a sequence that essentially says 'do foo' followed by 'do this incompatible thing that requires you didnt do the earlier thing we already said'.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,62 +186,102 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+ if (userObject instanceof Action) {
+ Action action = (Action) userObject;
+ action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ if (action.isVerbose()) {
+ context.out.print("Executing " + action.getClass().getName() + " ");
+ for (String arg : args) {
+ context.out.print(arg + " ");
+ }
+ context.out.println();
+ context.out.println("Home::" + action.getBrokerHome() + ", Instance::" + action.getBrokerInstance());
+ }
- if (action.isVerbose()) {
- context.out.print("Executing " + action.getClass().getName() + " ");
- for (String arg : args) {
- context.out.print(arg + " ");
+ return action.execute(context);
+ } else {
+ if (userObject instanceof Runnable) {
+ ((Runnable) userObject).run();
+ }
Review Comment:
Maybe do something if it isnt? Should it ever get here otherwise?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Create.java:
##########
@@ -712,11 +711,15 @@ public Object run(ActionContext context) throws Exception {
File logFolder = createDirectory("log", directory);
File oomeDumpFile = new File(logFolder, "oom_dump.hprof");
- if (javaOptions == null || javaOptions.length() == 0) {
- javaOptions = "";
+ StringBuffer javaOption = new StringBuffer();
Review Comment:
Same comment as other file. Same impl too...maybe add a helper.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -194,62 +186,102 @@ public static Object execute(boolean inputEnabled, boolean useSystemOut, File ar
* Useful on test cases
*/
private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
- return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
+ return internalExecute(artemisHome, artemisInstance, etcFolder, args, new ActionContext());
}
public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
- Action action = builder(artemisInstance).build().parse(args);
- action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ boolean isInstance = artemisInstance != null || System.getProperty("artemis.instance") != null;
+ CommandLine commandLine = buildCommand(isInstance, !isInstance);
+
+ Object userObject = parseAction(commandLine, args);
+ if (userObject instanceof Action) {
+ Action action = (Action) userObject;
+ action.setHomeValues(artemisHome, artemisInstance, etcFolder);
+ if (action.isVerbose()) {
+ context.out.print("Executing " + action.getClass().getName() + " ");
+ for (String arg : args) {
+ context.out.print(arg + " ");
+ }
+ context.out.println();
+ context.out.println("Home::" + action.getBrokerHome() + ", Instance::" + action.getBrokerInstance());
+ }
- if (action.isVerbose()) {
- context.out.print("Executing " + action.getClass().getName() + " ");
- for (String arg : args) {
- context.out.print(arg + " ");
+ return action.execute(context);
+ } else {
+ if (userObject instanceof Runnable) {
+ ((Runnable) userObject).run();
+ }
+ }
+ return null;
+ }
+
+ /*
+ Pico-cli traditionally would execute user objects that implement Runnable.
+ However as we used airline before, we needed parse for the proper action.
+ This method here is parsing the arg and find the proper user object in the hierarchy of sub-commands
+ and return it to the caller.
+ */
+ private static Object parseAction(CommandLine line, String[] args) {
+ CommandLine.ParseResult parseResult = line.parseArgs(args);
+ if (parseResult != null) {
+ for (; parseResult.hasSubcommand(); parseResult = parseResult.subcommand()) {
}
- context.out.println();
- context.out.println("Home::" + action.getBrokerHome() + ", Instance::" + action.getBrokerInstance());
}
+ if (parseResult == null) {
+ throw new RuntimeException("Cannot match arg::" + Arrays.toString(args));
+ }
+ return parseResult.commandSpec().userObject();
+ }
+
+ public static CommandLine buildCommand(boolean includeInstanceCommands, boolean includeHomeCommands) {
+ return buildCommand(includeInstanceCommands, includeHomeCommands, true);
- action.checkOptions(args);
- return action.execute(context);
}
- private static CliBuilder<Action> builder(File artemisInstance) {
- String instance = artemisInstance != null ? artemisInstance.getAbsolutePath() : System.getProperty("artemis.instance");
- CliBuilder<Action> builder = Cli.<Action>builder("artemis").withDescription("ActiveMQ Artemis Command Line").
- withCommand(HelpAction.class).withCommand(Producer.class).withCommand(Transfer.class).withCommand(Consumer.class).
- withCommand(Browse.class).withCommand(Mask.class).withCommand(PrintVersion.class).withDefaultCommand(HelpAction.class);
-
- builder.withGroup("perf").withDescription("Perf tools group (example ./artemis perf client)")
- .withDefaultCommand(PerfClientCommand.class)
- .withCommands(PerfProducerCommand.class, PerfConsumerCommand.class, PerfClientCommand.class);
-
- builder.withGroup("check").withDescription("Check tools group (node|queue) (example ./artemis check node)").
- withDefaultCommand(HelpCheck.class).withCommands(NodeCheck.class, QueueCheck.class);
-
- builder.withGroup("queue").withDescription("Queue tools group (create|delete|update|stat|purge) (example ./artemis queue create)").
- withDefaultCommand(HelpQueue.class).withCommands(CreateQueue.class, DeleteQueue.class, UpdateQueue.class, StatQueue.class, PurgeQueue.class);
-
- builder.withGroup("address").withDescription("Address tools group (create|delete|update|show) (example ./artemis address create)").
- withDefaultCommand(HelpAddress.class).withCommands(CreateAddress.class, DeleteAddress.class, UpdateAddress.class, ShowAddress.class);
-
- if (instance != null) {
- builder.withGroup("activation")
- .withDescription("activation tools group (sync) (example ./artemis activation list)")
- .withDefaultCommand(ActivationSequenceList.class)
- .withCommands(ActivationSequenceList.class, ActivationSequenceSet.class);
- builder.withGroup("data").withDescription("data tools group (print|imp|exp|encode|decode|compact|recover) (example ./artemis data print)").
- withDefaultCommand(HelpData.class).withCommands(RecoverMessages.class, PrintData.class, XmlDataExporter.class, XmlDataImporter.class, DecodeJournal.class, EncodeJournal.class, CompactJournal.class);
- builder.withGroup("user").withDescription("default file-based user management (add|rm|list|reset) (example ./artemis user list)").
- withDefaultCommand(HelpUser.class).withCommands(ListUser.class, AddUser.class, RemoveUser.class, ResetUser.class);
- builder = builder.withCommands(Run.class, Stop.class, Kill.class, PerfJournal.class);
- } else {
- builder.withGroup("data").withDescription("data tools group (print|recover) (example ./artemis data print)").
- withDefaultCommand(HelpData.class).withCommands(RecoverMessages.class, PrintData.class);
- builder = builder.withCommands(Create.class, Upgrade.class);
+ public static CommandLine buildCommand(boolean includeInstanceCommands, boolean includeHomeCommands, boolean includeShell) {
+ Artemis artemis = new Artemis();
+
+ CommandLine commandLine = new CommandLine(artemis);
+ artemis.setCommandLine(commandLine);
+
+ HelpAction help = new HelpAction();
+ help.setCommandLine(commandLine);
+ commandLine.addSubcommand(help);
+
+ commandLine.addSubcommand(new AutoCompletion());
+
+ if (includeShell) {
+ commandLine.addSubcommand(new Shell(commandLine));
+ }
+
+ commandLine.addSubcommand(new Producer()).addSubcommand(new Transfer()).addSubcommand(new Consumer()).addSubcommand(new Browse()).addSubcommand(new Mask()).addSubcommand(new PrintVersion());
+
+ commandLine.addSubcommand(new PerfGroup(commandLine));
+ commandLine.addSubcommand(new CheckGroup(commandLine));
+ commandLine.addSubcommand(new QueueGroup(commandLine));
+ commandLine.addSubcommand(new AddressGroup(commandLine));
+
+ if (includeInstanceCommands) {
+ commandLine.addSubcommand(new ActivationGroup(commandLine));
+ commandLine.addSubcommand(new DataGroup(commandLine));
+ commandLine.addSubcommand(new UserGroup(commandLine));
+
+ commandLine.addSubcommand(new Run());
+ commandLine.addSubcommand(new Stop());
+ commandLine.addSubcommand(new Kill());
+ commandLine.addSubcommand(new PerfJournal());
+ }
+
+ if (includeHomeCommands) {
+ if (!includeInstanceCommands) {
+ // Data is already preent in InstanceCommands
Review Comment:
typo: preent
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1279851195
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -157,68 +174,132 @@ private ConnectionFactory createAMQPConnectionFactory(String brokerURL,
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e.printStackTrace();
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
getActionContext().err.println("Connection failed::" + e.getMessage());
- cf = new JmsConnectionFactory(inputUser(user), inputPassword(password), inputBrokerURL(brokerURL));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new JmsConnectionFactory(user, password, brokerURL);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ e2.printStackTrace();
+ }
return cf;
}
}
protected ActiveMQConnectionFactory createCoreConnectionFactory() {
+ recoverConnectionInformation();
return createCoreConnectionFactory(brokerURL, user, password, clientID);
}
+ private void recoverConnectionInformation() {
+ if (CONNECTION_INFORMATION.get() != null) {
+ ConnectionInformation connectionInfo = CONNECTION_INFORMATION.get();
+ if (this.user == null) {
+ this.user = connectionInfo.user;
+ }
+ if (this.password == null) {
+ this.password = connectionInfo.password;
+ }
+ if (this.brokerURL == null) {
+ this.brokerURL = connectionInfo.uri;
+ }
+
+ }
+ }
+
+ void saveConnectionInfo(String brokerURL, String user, String password) {
+ if (Shell.inShell() && CONNECTION_INFORMATION.get() == null) {
+ CONNECTION_INFORMATION.set(new ConnectionInformation(brokerURL, user, password));
+ System.out.println("CLI connected to broker " + brokerURL + ", user:" + user);
+ }
+ }
+
protected ActiveMQConnectionFactory createCoreConnectionFactory(String brokerURL,
String user,
String password,
String clientID) {
+ if (brokerURL.startsWith("amqp://")) {
+ // replacing amqp:// by tcp://
+ brokerURL = "tcp" + brokerURL.substring(4);
+ }
+
ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
getActionContext().out.println("Consumer:: clientID = " + clientID);
cf.setClientID(clientID);
}
try {
- Connection connection = cf.createConnection();
- connection.close();
+ tryConnect(brokerURL, user, password, cf);
return cf;
} catch (JMSSecurityException e) {
// if a security exception will get the user and password through an input
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(brokerURL, inputUser(user), inputPassword(password));
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
return cf;
} catch (JMSException e) {
// if a connection exception will ask for the URL, user and password
if (getActionContext() != null) {
getActionContext().err.println("Connection failed::" + e.getMessage());
}
- cf = new ActiveMQConnectionFactory(inputBrokerURL(brokerURL), inputUser(user), inputPassword(password));
+ brokerURL = inputBrokerURL(brokerURL);
+ user = inputUser(user);
+ password = inputPassword(password);
+ cf = new ActiveMQConnectionFactory(brokerURL, user, password);
if (clientID != null) {
cf.setClientID(clientID);
}
+ try {
+ tryConnect(brokerURL, user, password, cf);
+ } catch (Exception e2) {
+ }
Review Comment:
not really in both cases.. This is the exception of the exception. IN case the input user/password does not succeed, if this does not work this will simply not save the password 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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1658502892
@gemmellr I am merging this... further changes can be sent into main after this.
Thanks for your help on this
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276299266
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java:
##########
@@ -106,8 +106,16 @@ public Object run(ActionContext context) throws Exception {
throw new IOException(etcFolder + " does not exist for etc");
}
+ StringBuffer javaOption = new StringBuffer();
Review Comment:
StringBuilder?
Name could be more obvious than the singular of the original plural list, e.g javaOptionsArgs...or javaOptionsString (and build the value here rather than later at the point it is passed, throw away the builder).
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1276667532
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/UserGroup.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.cli.commands.user;
+
+import org.apache.activemq.artemis.cli.commands.HelpAction;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "user", description = "file-based user management (example ./artemis user list)", subcommands = {ListUser.class, AddUser.class, RemoveUser.class, ResetUser.class})
Review Comment:
I think I can specify the description on the API and use the isShell that I implemented. will take a look.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277454004
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java:
##########
@@ -205,6 +205,20 @@ the ARTEMIS_HOME variable will include back slashes (An invalid file URI charact
return brokerHome;
}
+ @Override
+ public void run() {
+ try {
+ execute(getActionContext());
+ } catch (Throwable e) {
+ e.printStackTrace();
Review Comment:
the Shell will only call the Runnable. So this to interface back to the execute method.
--
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] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1655973410
Only thing left on my todo is the JDK parameters and testing with a newer version. will do it before the end of today.
--
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] clebertsuconic commented on pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#issuecomment-1654800574
I have added one more review from myself. When using user/password the system would ask me for the user/password all the time. I'm now saving the last successful user/password between executions, adding a connect and disconnect that's only available through the shell (what makes the shell more useful since it won't ask for user/password every time when using security)
--
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] gemmellr commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1277589970
##########
artemis-distribution/src/main/resources/licenses/bin/LICENSE:
##########
@@ -286,6 +286,12 @@ For HdrHistogram:
This product bundles HdrHistogram, which is available under the
"2-clause BSD" license. For details, see licences/LICENSE-hdrhistogram.txt.
+==============================================================================
+For JLine:
+==============================================================================
+This product bundles JLine, which is available under the
+"3-clause BSD" license. For details, see licences/LICENSE-jline.txt.
Review Comment:
Referenced file still needs added.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java:
##########
@@ -87,9 +68,29 @@
* Notice that this class should not use any logging as it's part of the bootstrap and using logging here could
* disrupt the order of bootstrapping on certain components (e.g. JMX being started from log4j)
*/
-public class Artemis {
+@Command(name = "artemis", description = "ActiveMQ Artemis Command Line")
+public class Artemis implements Runnable {
+
+ CommandLine commandLine;
+
+ public CommandLine getCommandLine() {
+ return commandLine;
+ }
+
+ public Artemis setCommandLine(CommandLine commandLine) {
+ this.commandLine = commandLine;
+ return this;
+ }
+
+ @Override
+ public void run() {
+ // We are running the shell by default.
+ // if you type ./artemis we will go straight to the shell
+ Shell.runShell();
+ }
public static void main(String... args) throws Exception {
+ System.out.println("Line::" + getNameFromBanner());
Review Comment:
Is this a leftover?
##########
docs/user-manual/en/using-server.md:
##########
@@ -53,6 +53,8 @@ The following highlights some important folders on the distribution:
## Creating a Broker Instance
+Before the broker is used, an broker *instance* must be created.
Review Comment:
an -> a
I would put the sentence at the start of the existing paragraph below about broker instances.
--
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] clebertsuconic commented on a diff in pull request #4565: ARTEMIS-4372 Implement Pico-cli and script auto-complete
Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4565:
URL: https://github.com/apache/activemq-artemis/pull/4565#discussion_r1278011424
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
@gnodet I"m adding jline3 into artemis, integrated with pico-CLI. This PR is taking an example from the jline3 along with some examples I saw on pico-cli .. and I had to include these with JDK-11, while these are not needed in 17.
I'm confused what I need to do to integrate this. can you help us here?
##########
artemis-distribution/src/main/resources/bin/artemis:
##########
@@ -91,6 +91,9 @@ if $cygwin ; then
fi
exec "$JAVACMD" $JAVA_ARGS $ARTEMIS_CLUSTER_PROPS \
+ --add-opens java.base/java.lang=ALL-UNNAMED \
+ --add-opens java.base/java.io=ALL-UNNAMED \
+ --add-opens java.base/java.util=ALL-UNNAMED \
Review Comment:
hmmmmmmm... I don't need these with JDK17.
@gnodet it would be so nice to get your help 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-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org