KNOX-3278: Update JLine to 3.30.6 - #1181
Conversation
Test Results46 tests 46 ✅ 6s ⏱️ Results for commit 9230a8a. ♻️ This comment has been updated with latest results. |
| <excludes> | ||
| <exclude>schema/**</exclude> | ||
| <exclude>**/*.ldif</exclude> | ||
| <exclude>META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat</exclude> |
There was a problem hiding this comment.
if we use maven-shade-plugin:3.4.1 and org.apache.logging.log4j:log4j-transform-maven-shade-plugin-extensions, this can be fixed by adding
<transformer implementation="org.apache.logging.log4j.maven.plugins.shade.transformer.Log4j2PluginCacheFileTransformer"/>
Otherwise the Log4j2Plugins.dat will be overwritten by our plugins and will not contain the ones shipped by log4j2.
| checkJava | ||
| buildAppJavaOpts | ||
| $JAVA "${APP_JAVA_OPTS[@]}" -Dlog4j.configurationFile=conf/knoxshell-log4j2.xml -javaagent:"$APP_BIN_DIR"/../lib/aspectjweaver.jar -cp "$APP_JAR":lib/* org.apache.knox.gateway.shell.Shell "$@" || exit 1 | ||
| $JAVA "${APP_JAVA_OPTS[@]}" -Dlog4j.configurationFile=conf/knoxshell-log4j2.xml -javaagent:"$APP_BIN_DIR"/../lib/aspectjweaver.jar -cp "$APP_JAR":lib/* -cp "$APP_JAR":lib/* org.apache.knox.gateway.launcher.Launcher "$@" || exit 1 |
There was a problem hiding this comment.
Why the change from shell.Shell to launcher.Launcher?
There was a problem hiding this comment.
I kept Launcher and removed the duplicate classpath option.
It's because our logging configuration is referring to sys:launcher.dir and sys:launcher.name.
The shaded jar is also using Launcher as main class.
Either we specify these properties in the launcher script or we use Launcher.
Otherwise the log file will not be created:
./bin/knoxshell.sh -e "org.apache.logging.log4j.LogManager.getLogger('test').error('hello from knoxshell')"
main ERROR Unable to create file ${sys:launcher.dir}/../logs/${sys:launcher.name}.log java.io.IOException: No such file or directory at java.base/java.io.UnixFileSystem.createFileExclusively(Native Method) at java.base/java.io.File.createNewFile(File.java:1043) at org.apache.logging.log4j.core.appender.rolling.RollingFileManager.lambda$getFileManager$0(RollingFileManager.java:315)
There are currently classloader issues if we would use -jar and we place extra jars, for example database driver jars in the lib folder. So the -cp flag is needed in that case and -jar cannot be used (the shaded jar does not specify a classpath in its manifest). I'll create a separate issue to use launcher and be consistent with other modules.
| KnoxLoginDialog dlg = new KnoxLoginDialog(); | ||
| dlg.collect(); | ||
| return dlg; | ||
| LineReader reader = LineReaderBuilder.builder() |
There was a problem hiding this comment.
This appears to be a re-implementation of the credential collector logic in the name of the JLine upgrade? Is it necessary for the dependency upgrade?
There was a problem hiding this comment.
You're right, it's not necessary. Reverted back to KnoxLoginDialog.
…hade-plugin and knoxshell-log4j. Update aspectj for JDK 17.
…is not supported. This will impact performance.
…nexpected exceptions.
…asses from the lib directory.
…ve duplicate classpath argument.
Reverts LoginCommand.execute() to use KnoxLoginDialog for credential collection instead of JLine3 terminal prompts, restoring the Swing GUI popup dialog behavior. This maintains consistency with the `:login` command behavior from before KNOX-3278 refactoring, as documented in the original pre-refactor version.
…uld otherwise not be changed.
…uld otherwise not be changed.
moresandeep
left a comment
There was a problem hiding this comment.
Thanks @bonampak! Looks good
* KNOX-3278: Update jline.version to 3.25.1. * KNOX-3278: Update groovy to 5.0.4 and jline to 3.30.6, plus jna to 5.18.1. * KNOX-3278: refactor commands and REPL shell. * KNOX-3278: fix checkstyle errors and logging configuration in maven-shade-plugin and knoxshell-log4j. Update aspectj for JDK 17. * KNOX-3278: Fix forbiddenapis check: use Locale.ROOT in printf. * KNOX-3278: Fix KnoxShellTableCallHistoryTest shouldRollbackToValidPreviousStep * KNOX-3278: Removing comments and unused methods * KNOX-3278: Fix missing newlines at end of file. * KNOX-3278: Restoring SelectCommand to use Swing JTextArea for multiline edits. * KNOX-3278: Removing KnoxLoginDialog (JLine3 supports password input). * KNOX-3278: Correcting undeclared jline module dependencies. * KNOX-3278: Correcting pmd findings. * KNOX-3278: Update rest-assured to 6.0.0 (needed for Groovy 5) * KNOX-3278: command completion pt1 * KNOX-3278: command completion pt2 * KNOX-3278: correct checkstyle errors * KNOX-3278: correct undeclared jline module dependencies. * KNOX-3278: cleanup DataSourceCommand completer. * KNOX-3278: add :x and :q as exit commands. * KNOX-3278: add '?' as help command alias. * KNOX-3278: import, load and show commands added. * KNOX-3278: purge command added. * KNOX-3278: Correcting load command and alias. * KNOX-3278: Adding completer for ShowCommand. * KNOX-3278: Correct checkstyle error on shortcut handling * KNOX-3278: Renaming SimpleCommandRegistry to KnoxShellCommandRegistry and cleaning up command handling. Overriding name() so that tab completion after semicolon does not show the name of the command registry. * KNOX-3278: No need to exclude Log4j2Plugins.dat from the shaded knoxhsell jar. * KNOX-3278: correcting knoxshell.sh to use -jar instead of main class (to use Launcher taken from knoxshell.jar manifest main class) * KNOX-3278: excluding Log4j2Plugins.dat from the shaded knoxhsell jar and making knoxshell-log4j2.xml consistent with other log4j2 configurations. * KNOX-3278: get rid of useless warning javax.* types are not being woven * KNOX-3278: fix log4j2 WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance. * KNOX-3278: Fix purge command, simplify import and show commands. * KNOX-3278: Fix ImportCommand and add support for static imports. * KNOX-3278: Catch Throwable similarly to legacy GroovySh. * KNOX-3278: Make LoadCommand handle multi-file loading as in legacy GroovySh 4. * KNOX-3278: wrap GroovyEngine completer into SafeCompleter to handle unexpected exceptions. * KNOX-3278: correct knoxshell.sh to use launcher and correctly load classes from the lib directory. * KNOX-3278: fix no endline at end of file. * KNOX-3278: refactor Shell.java pt1 * KNOX-3278: refactor Shell.java pt2 * KNOX-3238: review findings: keep Launcher because of logging and remove duplicate classpath argument. * KNOX-3278: review findings: Restore KnoxLoginDialog for credential collection logic. * KNOX-3278: review findings: Revert LoginCommand to use KnoxLoginDialog. Reverts LoginCommand.execute() to use KnoxLoginDialog for credential collection instead of JLine3 terminal prompts, restoring the Swing GUI popup dialog behavior. This maintains consistency with the `:login` command behavior from before KNOX-3278 refactoring, as documented in the original pre-refactor version. * KNOX-3278: review findings: Replace inline FQN with import * KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed. * KNOX-3278: review findings: remove override for KnoxShellCommandRegistry.name() * KNOX-3278: correcting load command to parse filenames with spaces as well * KNOX-3278: correcting Groovy 5 extension module registration. * KNOX-3278: Ctrl+C continues back to the prompt; Ctrl+D exits the shell. * KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed. * KNOX-3278: review findings: fix purge command javadoc and help.
* KNOX-3278: Update jline.version to 3.25.1. * KNOX-3278: Update groovy to 5.0.4 and jline to 3.30.6, plus jna to 5.18.1. * KNOX-3278: refactor commands and REPL shell. * KNOX-3278: fix checkstyle errors and logging configuration in maven-shade-plugin and knoxshell-log4j. Update aspectj for JDK 17. * KNOX-3278: Fix forbiddenapis check: use Locale.ROOT in printf. * KNOX-3278: Fix KnoxShellTableCallHistoryTest shouldRollbackToValidPreviousStep * KNOX-3278: Removing comments and unused methods * KNOX-3278: Fix missing newlines at end of file. * KNOX-3278: Restoring SelectCommand to use Swing JTextArea for multiline edits. * KNOX-3278: Removing KnoxLoginDialog (JLine3 supports password input). * KNOX-3278: Correcting undeclared jline module dependencies. * KNOX-3278: Correcting pmd findings. * KNOX-3278: Update rest-assured to 6.0.0 (needed for Groovy 5) * KNOX-3278: command completion pt1 * KNOX-3278: command completion pt2 * KNOX-3278: correct checkstyle errors * KNOX-3278: correct undeclared jline module dependencies. * KNOX-3278: cleanup DataSourceCommand completer. * KNOX-3278: add :x and :q as exit commands. * KNOX-3278: add '?' as help command alias. * KNOX-3278: import, load and show commands added. * KNOX-3278: purge command added. * KNOX-3278: Correcting load command and alias. * KNOX-3278: Adding completer for ShowCommand. * KNOX-3278: Correct checkstyle error on shortcut handling * KNOX-3278: Renaming SimpleCommandRegistry to KnoxShellCommandRegistry and cleaning up command handling. Overriding name() so that tab completion after semicolon does not show the name of the command registry. * KNOX-3278: No need to exclude Log4j2Plugins.dat from the shaded knoxhsell jar. * KNOX-3278: correcting knoxshell.sh to use -jar instead of main class (to use Launcher taken from knoxshell.jar manifest main class) * KNOX-3278: excluding Log4j2Plugins.dat from the shaded knoxhsell jar and making knoxshell-log4j2.xml consistent with other log4j2 configurations. * KNOX-3278: get rid of useless warning javax.* types are not being woven * KNOX-3278: fix log4j2 WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance. * KNOX-3278: Fix purge command, simplify import and show commands. * KNOX-3278: Fix ImportCommand and add support for static imports. * KNOX-3278: Catch Throwable similarly to legacy GroovySh. * KNOX-3278: Make LoadCommand handle multi-file loading as in legacy GroovySh 4. * KNOX-3278: wrap GroovyEngine completer into SafeCompleter to handle unexpected exceptions. * KNOX-3278: correct knoxshell.sh to use launcher and correctly load classes from the lib directory. * KNOX-3278: fix no endline at end of file. * KNOX-3278: refactor Shell.java pt1 * KNOX-3278: refactor Shell.java pt2 * KNOX-3238: review findings: keep Launcher because of logging and remove duplicate classpath argument. * KNOX-3278: review findings: Restore KnoxLoginDialog for credential collection logic. * KNOX-3278: review findings: Revert LoginCommand to use KnoxLoginDialog. Reverts LoginCommand.execute() to use KnoxLoginDialog for credential collection instead of JLine3 terminal prompts, restoring the Swing GUI popup dialog behavior. This maintains consistency with the `:login` command behavior from before KNOX-3278 refactoring, as documented in the original pre-refactor version. * KNOX-3278: review findings: Replace inline FQN with import * KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed. * KNOX-3278: review findings: remove override for KnoxShellCommandRegistry.name() * KNOX-3278: correcting load command to parse filenames with spaces as well * KNOX-3278: correcting Groovy 5 extension module registration. * KNOX-3278: Ctrl+C continues back to the prompt; Ctrl+D exits the shell. * KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed. * KNOX-3278: review findings: fix purge command javadoc and help. (cherry picked from commit 114af08)
KNOX-3278 - Update jline to 3.30.6
What changes were proposed in this pull request?
Update groovy to 5.0.4, jline to 3.30.6, jna to 5.18.1, aspectj to 1.9.25.1 and rest-assured to 6.0.0.
Rewrote gateway-shell using JLine 3 and Groovy 5. As
How was this patch tested?
Ran example tests and all repl commands (ExampleWebHdfsLs.groovy and ExampleManagerResourceDeployment.groovy both in REPL with :load and as knoxshell.sh arguments. Both work as expected.)
Build:
mvn -Prelease,package clean install -am -pl gateway-shell,gateway-shell-releaseSetup:
target/3.0.0-SNAPSHOT/knoxshell-3.0.0-SNAPSHOT.zipto a directory.postgresql-42.7.10.jartoknoxshell-3.0.0-SNAPSHOT/lib/Tests
Logging
Datasources command
Prerequisites:
fs command
Invoking groovy scripts with knoxshell
Prerequisite: hdfs home directory for your user exists on the cluster.
On the cluster, verified the put was successful:
Legacy Groovysh commands
Commands skipped (not applicable to CLI/JLine3 architecture and current feature set):
:display, :inspect, :edit, :record, :alias, :set, :register, :doc, :grab
Also not implemented:
:save / :s — dump session history/buffer to a file
:history (jline3 history file will be in ~/.knoxshell_history)
Implemented:
:import, :show, :load, :purge (also for old :clear)
Tested filenames with spaces:
inside the shell, only the quoted one works:
Integration Tests
There is another ticket for adding integration tests:
KNOX-3280 Add integration tests for KnoxShell and KnoxCLI
UI changes
None