-
Notifications
You must be signed in to change notification settings - Fork 467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WFCORE-6895] Add a grace period to Management CLI Installer Windows … #6071
Conversation
@@ -89,3 +89,6 @@ if ($HOST_CONTROLLER_JAVA_OPTS -eq $null) { | |||
# Sample JPDA settings for shared memory debugging | |||
#PROCESS_CONTROLLER_JAVA_OPTS="$PROCESS_CONTROLLER_JAVA_OPTS -agentlib:jdwp=transport=dt_shmem,server=y,suspend=n,address=jboss" | |||
#HOST_CONTROLLER_JAVA_OPTS="$HOST_CONTROLLER_JAVA_OPTS -agentlib:jdwp=transport=dt_shmem,server=y,suspend=n,address=jboss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not connected to this PR, but should those assignments start with $sign
(e.g. $HOST_CONTROLLER_JAVA_OPTS
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!, variables in Powershell are defined starting the with $, I'll append a commit
@@ -177,11 +176,6 @@ protected void doHandle(CommandContext ctx) throws CommandLineException { | |||
|
|||
if(isLocalClient) { | |||
ctx.printLine("The JBoss CLI session will be closed automatically to allow the server be updated. Once the server has been restarted, you can relaunch the JBoss CLI session.", false); | |||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wait in the startup scripts only applies to Windows scripts, but this wait happened also on Linux - is that OK to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spyrkob thanks for commenting on it. It does nothing, it just waits a bit to have a better UX; you execute the command and see how it waits a bit with the message printed, that's all. It is safe to close it immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of questions below, otherwise LGTM
Thanks @spyrkob |
…script
Jira issue: https://issues.redhat.com/browse/WFCORE-6895
This just adds a configurable grace period to help prevent locking file errors on Windows when using Management CLI Installer