tomaswolf commented on code in PR #726:
URL: https://github.com/apache/mina-sshd/pull/726#discussion_r2038905778
##########
sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java:
##########
@@ -281,19 +282,25 @@ default String executeRemoteCommand(String command,
OutputStream stderr, Charset
/**
* Execute a command that requires no input and redirects its
STDOUT/STDERR streams to the user-provided ones
*
- * @param command The command to execute - without a terminating LF
- * @param stdout Standard output stream - if {@code null} then
stream data is ignored. <B>Note:</B> if the
- * stream is not {@code null} then it will be left
<U>open</U> when this method returns or
- * exception is thrown
- * @param stderr Error output stream - if {@code null} then stream
data is ignored. <B>Note:</B> if the stream
- * is not {@code null} then it will be left
<U>open</U> when this method returns or exception is
- * thrown
- * @param charset The command {@link Charset} for output/error - if
{@code null} then US_ASCII is assumed
- * @throws IOException If failed to execute the command or got a non-zero
exit status
- * @see ClientChannel#validateCommandExitStatusCode(String,
Integer) validateCommandExitStatusCode
+ * @param command The command to execute - without a
terminating LF.
+ * @param stdout Standard output stream - if {@code null}
then stream data is ignored.
+ * <b>Note:</b> if the stream is not {@code
null}, it will be left <u>open</u> when this
+ * method returns or an exception is thrown.
+ * @param stderr Error output stream - if {@code null} then
error stream data is ignored.
+ * <b>Note:</b> if the stream is not {@code
null}, it will be left <u>open</u> when this
+ * method returns or an exception is thrown.
+ * @param charset The charset to use for encoding the command
and decoding the output/error streams.
+ * If {@code null}, US-ASCII is assumed.
+ * @param commandTimeoutMillis Timeout (in milliseconds) for the remote
command execution. Applies to both
+ * channel opening and result waiting. A zero
or negative value means no timeout.
+ * @throws IOException If the command execution fails, times out,
or returns a non-zero exit code.
+ * A {@link RemoteException} may be thrown if
the remote side reports an error.
+ * @see ClientChannel#open()#verify(long,
java.util.concurrent.TimeUnit)
+ * @see ClientChannel#waitFor(Collection, long)
+ * @see
ClientChannel#validateCommandExitStatusCode(String, Integer)
validateCommandExitStatusCode
*/
default void executeRemoteCommand(
- String command, OutputStream stdout, OutputStream stderr, Charset
charset)
+ String command, OutputStream stdout, OutputStream stderr, Charset
charset, long commandTimeoutMillis)
Review Comment:
This is an API change and is thus a no-go.
If you want this, you'll have to add a new method, and make the existing one
call the new one.
##########
sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java:
##########
@@ -304,10 +311,10 @@ default void executeRemoteCommand(
ClientChannel channel = createExecChannel(command, charset, null,
Collections.emptyMap())) {
channel.setOut(channelOut);
channel.setErr(channelErr);
- channel.open().await(); // TODO use verify and a configurable
timeout
- // TODO use a configurable timeout
- Collection<ClientChannelEvent> waitMask =
channel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, 0L);
+ channel.open().verify(commandTimeoutMillis, TimeUnit.MICROSECONDS);
+
+ Collection<ClientChannelEvent> waitMask =
channel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, commandTimeoutMillis);
Review Comment:
Here we should probably wait only for commandTimeoutMillis minus the time we
spent waiting for the channel to open.
##########
sshd-core/src/main/java/org/apache/sshd/client/session/ClientSession.java:
##########
@@ -304,10 +311,10 @@ default void executeRemoteCommand(
ClientChannel channel = createExecChannel(command, charset, null,
Collections.emptyMap())) {
channel.setOut(channelOut);
channel.setErr(channelErr);
- channel.open().await(); // TODO use verify and a configurable
timeout
- // TODO use a configurable timeout
- Collection<ClientChannelEvent> waitMask =
channel.waitFor(REMOTE_COMMAND_WAIT_EVENTS, 0L);
+ channel.open().verify(commandTimeoutMillis, TimeUnit.MICROSECONDS);
Review Comment:
Time unit mismatch (millis vs micros).
--
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: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
