Skip to content

KNOX-3158: Enhancing the maintainability of knox-shell #1056

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

KNOX-3158: Enhancing the maintainability of knox-shell #1056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/commands/DataSourceCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;

import org.apache.knox.gateway.shell.CredentialCollectionException;
import org.apache.knox.gateway.shell.CredentialCollector;
Expand All @@ -37,6 +38,8 @@ public DataSourceCommand(Groovysh shell) {
super(shell, ":datasources", ":ds", DESC, USAGE, DESC);
}

Logger logger = Logger.getLogger(getClass().getName());

@SuppressWarnings({"unchecked", "PMD.CloseResource"})
@Override
public Object execute(List<String> args) {
Expand All @@ -63,12 +66,12 @@ else if (args.get(0).equalsIgnoreCase("remove")) {
if (getVariables().get(KNOXDATASOURCE) != null) {
if (args.get(1) != null) {
if (((String)getVariables().get(KNOXDATASOURCE)).equals(args.get(1))) {
System.out.println("unselecting datasource.");
logger.info("unselecting datasource.");
getVariables().put(KNOXDATASOURCE, "");
}
}
else {
System.out.println("Missing datasource name to remove.");
logger.info("Missing datasource name to remove.");
}
}
getVariables().put(KNOXDATASOURCES, dataSources);
Expand Down
7 changes: 5 additions & 2 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/commands/KnoxLoginDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
*/
package org.apache.knox.gateway.shell.commands;

import java.util.logging.Logger;

import javax.swing.Box;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
Expand Down Expand Up @@ -98,11 +100,12 @@ public void setName(String name) {

public static void main(String[] args) {
KnoxLoginDialog dlg = new KnoxLoginDialog();
Logger logger = Logger.getLogger(KnoxLoginDialog.class.getName());
try {
dlg.collect();
if (dlg.ok) {
System.out.println("username: " + dlg.username);
System.out.println("password: " + new String(dlg.pass));
logger.info("username: " + dlg.username);
logger.info("password: " + new String(dlg.pass));
}
} catch (CredentialCollectionException e) {
e.printStackTrace();
Expand Down
7 changes: 5 additions & 2 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/commands/SelectCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.List;
import java.util.logging.Logger;
import java.util.Map;

import javax.swing.Box;
Expand All @@ -50,6 +51,8 @@ public SelectCommand(Groovysh shell) {
super(shell, ":SQL", ":sql", DESC, USAGE, DESC);
}

Logger logger = Logger.getLogger(getClass().getName());

@Override
public void keyPressed(KeyEvent event) {
int code = event.getKeyCode();
Expand Down Expand Up @@ -145,7 +148,7 @@ else if (dataSources.size() == 1) {
//KnoxShellTable.builder().jdbc().connect("jdbc:derby:codejava/webdb1").driver("org.apache.derby.jdbc.EmbeddedDriver").username("lmccay").pwd("xxxx").sql("SELECT * FROM book");
try {
if (ok) {
System.out.println(sql);
logger.info(sql);
try {
Connection conn = getConnectionFromSession(ds);
if (conn == null || conn.isClosed()) {
Expand All @@ -167,7 +170,7 @@ else if (dataSources.size() == 1) {
}
}
catch (SQLException e) {
System.out.println("SQL Exception encountered... " + e.getMessage());
logger.info("SQL Exception encountered... " + e.getMessage());
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/commands/WebHDFSCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.logging.Logger;
import java.util.Map;
import java.util.TimeZone;

Expand Down Expand Up @@ -58,6 +59,8 @@ public WebHDFSCommand(Groovysh shell) {
super(shell, ":filesystem", ":fs", DESC, USAGE, DESC);
}

Logger logger = Logger.getLogger(getClass().getName());

@Override
public Object execute(List<String> args) {
Map<String, String> mounts = getMountPoints();
Expand Down Expand Up @@ -139,8 +142,8 @@ else if (args.get(0).equalsIgnoreCase("get")) {
}
}
else {
System.out.println("Unknown filesystem command");
System.out.println(getUsage());
logger.info("Unknown filesystem command");
logger.info(getUsage());
}
return "";
}
Expand Down Expand Up @@ -350,7 +353,7 @@ private KnoxSession establishSession(String mountPoint, String url) {
private String collectClearInput(String prompt) {
Console c = System.console();
if (c == null) {
System.err.println("No console.");
logger.info("No console.");
System.exit(1);
}

Expand Down
43 changes: 23 additions & 20 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/jdbc/KnoxLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.HashMap;
import java.util.logging.Logger;
import java.util.Map;

import org.apache.knox.gateway.shell.CredentialCollectionException;
Expand All @@ -37,14 +38,16 @@ public class KnoxLine {
private KnoxDataSource datasource;
private Connection conn;

Logger logger = Logger.getLogger(getClass().getName());

@SuppressWarnings("PMD.DoNotUseThreads") // we need to define a Thread to be able to register a shutdown hook
public void execute(String[] args)
throws ClassNotFoundException, SQLException, CredentialCollectionException {

Runtime.getRuntime().addShutdownHook(new Thread() {
@Override
public void run() {
System.out.println("Closing any open connections ...");
logger.info("Closing any open connections ...");
closeConnection();
}
});
Expand All @@ -54,14 +57,14 @@ public void run() {
private void executeShell() {
String sql;

System.out.println(" _ _ _ ");
System.out.println("| | ___ __ _____ _| (_)_ __ ___ ");
System.out.println("| |/ / '_ \\ / _ \\ \\/ / | | '_ \\ / _ \\");
System.out.println("| <| | | | (_) > <| | | | | | __/");
System.out.println("|_|\\_\\_| |_|\\___/_/\\_\\_|_|_| |_|\\\\__|");
System.out.println("powered by Apache Knox");
System.out.println("");
System.out.println("");
logger.info(" _ _ _ ");
logger.info("| | ___ __ _____ _| (_)_ __ ___ ");
logger.info("| |/ / '_ \\ / _ \\ \\/ / | | '_ \\ / _ \\");
logger.info("| <| | | | (_) > <| | | | | | __/");
logger.info("|_|\\_\\_| |_|\\___/_/\\_\\_|_|_| |_|\\\\__|");
logger.info("powered by Apache Knox");
logger.info("");
logger.info("");

while(true) {
sql = System.console().readLine("knoxline> ");
Expand All @@ -76,25 +79,25 @@ private void executeShell() {
else {
// Configure JDBC connection
if (datasource != null) {
System.out.println(sql);
logger.info(sql);
try {
establishConnection();
try (Statement statement = conn.createStatement()) {
if (statement.execute(sql)) {
try (ResultSet resultSet = statement.getResultSet()) {
KnoxShellTable table = KnoxShellTable.builder().jdbc().resultSet(resultSet);
System.out.println(table.toString());
System.out.println("\nRows: " + table.getRows().size() + "\n");
logger.info(table.toString());
logger.info("\nRows: " + table.getRows().size() + "\n");
}
}
}
}
catch (SQLException e) {
System.out.println("SQL Exception encountered... " + e.getMessage());
logger.info("SQL Exception encountered... " + e.getMessage());
}
}
else {
System.out.println("No datasource selected. Use :ds select {datasource-name}");
logger.info("No datasource selected. Use :ds select {datasource-name}");
}
}
}
Expand All @@ -103,7 +106,7 @@ private void executeShell() {

private void establishConnection() throws SQLException {
if (conn == null || conn.isClosed()) {
System.out.println("Connecting...");
logger.info("Connecting...");
conn = JDBCUtils.createConnection(datasource.getConnectStr(), user, pass);
}
}
Expand All @@ -121,26 +124,26 @@ else if (args[1].equals("add")) {
addDataSource(args[2], args[3], args[4], args[5]);
}
else {
System.out.println("Invalid number of arguments for :ds add. Useage: :ds add {ds-name} {connectStr} {driver} {authnType: none|basic}");
logger.info("Invalid number of arguments for :ds add. Useage: :ds add {ds-name} {connectStr} {driver} {authnType: none|basic}");
}
}
else if (args[1].contentEquals("remove")) {
if (args.length == 3) {
removeDataSource(args[2]);
}
else {
System.out.println("Invalid number of arguments for :ds remove. Useage: :ds remove {ds-name}");
logger.info("Invalid number of arguments for :ds remove. Useage: :ds remove {ds-name}");
}
}
}

private void listDataSources() {
Map<String, KnoxDataSource> sources = getDataSources();
if (sources != null) {
sources.forEach((name, ds)->System.out.println("Name : " + name + " : " + ds.getConnectStr()));
sources.forEach((name, ds)->logger.info("Name : " + name + " : " + ds.getConnectStr()));
}
else {
System.out.println("No datasources configured. Use :ds add {ds-name} {connectStr} {driver} {authnType: none|basic}");
logger.info("No datasources configured. Use :ds add {ds-name} {connectStr} {driver} {authnType: none|basic}");
}
}

Expand Down Expand Up @@ -174,7 +177,7 @@ private void selectDataSource(String name) throws CredentialCollectionException
datasource = sources.get(name);

if (datasource == null) {
System.out.println("Invalid datasource name provided. See output from: :ds list");
logger.info("Invalid datasource name provided. See output from: :ds list");
return;
}

Expand Down
5 changes: 3 additions & 2 deletions gateway-shell/src/main/java/org/apache/knox/gateway/shell/jdbc/derby/DerbyDatabase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.logging.Logger;

import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.shell.jdbc.Database;
Expand All @@ -35,7 +36,7 @@ public class DerbyDatabase implements Database {
private static final String DEFAULT_SCHEMA_NAME = "APP";

private final String dbUri;

Logger logger = Logger.getLogger(getClass().getName());
/**
* Constructor
*
Expand Down Expand Up @@ -128,6 +129,6 @@ private void log(MessageLevel logLevel, String message) {

// being a test only class we now log into the STDOUT for now
private void log(MessageLevel logLevel, String message, Throwable error) {
System.out.println(logLevel.name() + " - " + message + (error == null ? "" : " - caused by " + error));
logger.info(logLevel.name() + " - " + message + (error == null ? "" : " - caused by " + error));
}
}
10 changes: 7 additions & 3 deletions ...ay-shell/src/main/java/org/apache/knox/gateway/shell/table/JDBCKnoxShellTableBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Locale;
import java.util.logging.Logger;

import org.apache.knox.gateway.shell.jdbc.JDBCUtils;

Expand Down Expand Up @@ -76,16 +77,19 @@ public JDBCKnoxShellTableBuilder driver(String driver) throws Exception {
}

private void loadDriver() throws Exception {

Logger logger = Logger.getLogger(getClass().getName());

try {
Class.forName(driver).newInstance();
} catch (ClassNotFoundException e) {
System.out.println(String.format(Locale.US, "Unable to load the JDBC driver %s. Check your CLASSPATH.", driver));
logger.info(String.format(Locale.US, "Unable to load the JDBC driver %s. Check your CLASSPATH.", driver));
throw e;
} catch (InstantiationException e) {
System.out.println(String.format(Locale.US, "Unable to instantiate the JDBC driver %s", driver));
logger.info(String.format(Locale.US, "Unable to instantiate the JDBC driver %s", driver));
throw e;
} catch (IllegalAccessException e) {
System.out.println(String.format(Locale.US, "Not allowed to access the JDBC driver %s", driver));
logger.info(String.format(Locale.US, "Not allowed to access the JDBC driver %s", driver));
throw e;
}
}
Expand Down
5 changes: 4 additions & 1 deletion gateway-shell/src/main/java/org/apache/knox/gateway/shell/table/KnoxShellTableRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.logging.Logger;
import java.util.Map;

import org.apache.commons.lang3.StringUtils;
Expand All @@ -34,6 +35,8 @@ class KnoxShellTableRenderer {

private final KnoxShellTable tableToRender;

Logger logger = Logger.getLogger(getClass().getName());

KnoxShellTableRenderer(KnoxShellTable tableToRender) {
this.tableToRender = tableToRender;
}
Expand Down Expand Up @@ -69,7 +72,7 @@ String toCSV(String filePath) {
try {
KnoxShellTableFileUtils.persistToFile(filePath, content);
} catch (IOException e) {
System.out.println("Persistence of CSV file has failed. " + e.getMessage());
logger.info("Persistence of CSV file has failed. " + e.getMessage());
e.printStackTrace();
}
}
Expand Down
Loading