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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
