[
https://issues.apache.org/jira/browse/ARTEMIS-4372?focusedWorklogId=873858&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-873858
]
ASF GitHub Bot logged work on ARTEMIS-4372:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 31/Jul/23 16:42
Start Date: 31/Jul/23 16:42
Worklog Time Spent: 10m
Work Description: 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.
Issue Time Tracking
-------------------
Worklog Id: (was: 873858)
Time Spent: 6h 10m (was: 6h)
> Move CLI framework to picocli and implement auto-complete
> ---------------------------------------------------------
>
> Key: ARTEMIS-4372
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4372
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Reporter: Clebert Suconic
> Assignee: Clebert Suconic
> Priority: Major
> Fix For: 2.31.0
>
> Time Spent: 6h 10m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
