You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2020/01/20 16:45:31 UTC
[GitHub] [knox] lmccay opened a new pull request #241: KNOX-1742 - Simple
SQL Client in KnoxShell for access to JDBC sources
lmccay opened a new pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241
(It is very **important** that you created an Apache Knox JIRA for this change and that the PR title/commit message includes the Apache Knox JIRA ID!)
## What changes were proposed in this pull request?
Adding a simple SQL client/shell to KnoxShell for accessing JDBC data sources such as HiveServer2 and others. Rather than providing connection details on the command line for the data source, this shell leverages the DataSource persistence introduced in KNOX-2128.
Enter the SQL client, list datasources, select datasources and execute SQL query:
lmccay@strange:~/Projects/releases/knoxshell-1.4.0-SNAPSHOT$ bin/knoxshell.sh knoxline
_ _ _
| | ___ __ _____ _| (_)_ __ ___
| |/ / '_ \ / _ \ \/ / | | '_ \ / _ \
| <| | | | (_) > <| | | | | | __/
|_|\_\_| |_|\___/_/\_\_|_|_| |_|\\__|
powered by Apache Knox
knoxline> :ds
Name : secgov : jdbc:hive2://sme-secgov-de-01.sme-secg.a465-9q4k.cloudera.site:8443/;ssl=true;transportMode=http;httpPath=sme-secgov-de-01/cdp-proxy-api/hive
Name : anatva : jdbc:hive2://de-anatva-master0.sandbox.a465-9q4k.cloudera.site/;ssl=true;transportMode=http;httpPath=de-anatva/cdp-proxy-api/hive
Name : derby1 : jdbc:derby:codejava/webdb1
knoxline> :ds select derby1
knoxline> select * from book
select * from book
BOOK
+------------+--------------------+
| BOOK_ID | TITLE |
+------------+--------------------+
| 1 | Effective Java |
| 2 | Core Java |
| 3 | Core Apache Knox |
+------------+--------------------+
Rows: 3
knoxline> ^CClosing any open connections ...
## How was this patch tested?
Manual testing of this feature and unit testing run to ensure no regressions.
Please review [Knox Contributing Process](https://cwiki.apache.org/confluence/display/KNOX/Contribution+Process#ContributionProcess-GithubWorkflow) before opening a pull request.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r368778336
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/jdbc/KnoxLine.java
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.shell.jdbc;
+
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.knox.gateway.shell.CredentialCollectionException;
+import org.apache.knox.gateway.shell.Credentials;
+import org.apache.knox.gateway.shell.KnoxDataSource;
+import org.apache.knox.gateway.shell.KnoxSession;
+import org.apache.knox.gateway.shell.table.KnoxShellTable;
+
+public class KnoxLine {
+ String user;
+ String pass;
+ KnoxDataSource datasource;
+ Connection conn;
+
+ 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 ...");
+ closeConnection();
+ }
+ });
+ executeShell();
+ }
+
+ 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("");
+
+ while(true) {
+ sql = System.console().readLine("knoxline> ");
+ if (sql != null && !sql.isEmpty()) {
+ if (sql.startsWith(":ds") || sql.startsWith(":datasource")) {
+ try {
+ processDataSourceCommand(sql);
+ } catch (CredentialCollectionException | SQLException e) {
+ e.printStackTrace();
+ }
+ }
+ else {
+ // Configure JDBC connection
+ if (datasource != null) {
+ System.out.println(sql);
+ 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");
+ }
+ }
+ }
+ catch(SQLException e) {
+ //e.printStackTrace()
+ System.out.println("SQL Exception encountered... " + e.getMessage());
+ // if (e.getMessage().contains("org.apache.thrift.transport.TTransportException")) {
+ // System.out.println("reconnecting... ");
+ // connection = DriverManager.getConnection( connectionString, user, pass );
Review comment:
Commented out code?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] smolnar82 commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r368891437
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/jdbc/KnoxLine.java
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.shell.jdbc;
+
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.knox.gateway.shell.CredentialCollectionException;
+import org.apache.knox.gateway.shell.Credentials;
+import org.apache.knox.gateway.shell.KnoxDataSource;
+import org.apache.knox.gateway.shell.KnoxSession;
+import org.apache.knox.gateway.shell.table.KnoxShellTable;
+
+public class KnoxLine {
+ String user;
+ String pass;
+ KnoxDataSource datasource;
+ Connection conn;
+
+ public void execute(String[] args)
Review comment:
You can ignore PMD's `DoNotUseThread` rule here like this.
```
@SuppressWarnings("PMD.DoNotUseThreads") //we need to define a Thread to be able to register a shutdown hook
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] lmccay merged pull request #241: KNOX-1742 - Simple SQL
Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
lmccay merged pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r368777946
##########
File path: build-tools/src/main/resources/build-tools/pmd/pmd-ruleset.xml
##########
@@ -88,6 +88,7 @@ limitations under the License.
<rule ref="category/java/multithreading.xml">
<exclude name="AvoidSynchronizedAtMethodLevel" />
<exclude name="UseConcurrentHashMap" />
+ <exclude name="DoNotUseThreads" />
Review comment:
Don't do this. There is a way to ignore specific methods only. We really shouldn't be creating threads especially in Jetty.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r368778454
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
##########
@@ -593,21 +594,42 @@ public String toString() {
String s = JsonUtils.renderAsJsonString(map);
String home = System.getProperty("user.home");
try {
- write(new File(home + File.separator + ".knoxshell" + File.separator + fileName), s);
+ write(new File(
+ home + File.separator + ".knoxshell" + File.separator + fileName),
+ s, StandardCharsets.UTF_8);
} catch (IOException e) {
e.printStackTrace();
}
}
- private static void write(File file, String s) throws IOException {
+ /**
+ * Persist provided Map to a file within the {user.home}/.knoxshell directory
+ * @param <T>
+ * @param fileName of persisted file
+ * @param map to persist
+ */
+ public static <T> void persistDataSourcesToKnoxShell(String fileName, Map<String, T> map) {
+ String s = JsonUtils.renderAsJsonString(map);
+ String home = System.getProperty("user.home");
+ try {
+ write(new File(
+ home + File.separator +
+ ".knoxshell" + File.separator + fileName),
+ s, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ }
+
+ private static void write(File file, String s, Charset utf8) throws IOException {
Review comment:
Don't need to expose `Charset`. UTF8 is correct in every case so far.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369100671
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
##########
@@ -593,7 +593,26 @@ public String toString() {
String s = JsonUtils.renderAsJsonString(map);
String home = System.getProperty("user.home");
try {
- write(new File(home + File.separator + ".knoxshell" + File.separator + fileName), s);
+ write(new File(
+ home + File.separator + ".knoxshell" + File.separator + fileName), s);
Review comment:
Just a note for future reference:
This could be `Paths.get(home, ".knoxshell", fileName).toFile()` or just use the `Path` directly instead of manually constructing a `File` object.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] lmccay commented on a change in pull request #241: KNOX-1742
- Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369096546
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/jdbc/KnoxLine.java
##########
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.shell.jdbc;
+
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.knox.gateway.shell.CredentialCollectionException;
+import org.apache.knox.gateway.shell.Credentials;
+import org.apache.knox.gateway.shell.KnoxDataSource;
+import org.apache.knox.gateway.shell.KnoxSession;
+import org.apache.knox.gateway.shell.table.KnoxShellTable;
+
+public class KnoxLine {
+ String user;
+ String pass;
+ KnoxDataSource datasource;
+ Connection conn;
+
+ public void execute(String[] args)
Review comment:
Thanks, Sandor!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] lmccay commented on a change in pull request #241: KNOX-1742
- Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369131838
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
##########
@@ -593,7 +593,26 @@ public String toString() {
String s = JsonUtils.renderAsJsonString(map);
String home = System.getProperty("user.home");
try {
- write(new File(home + File.separator + ".knoxshell" + File.separator + fileName), s);
+ write(new File(
+ home + File.separator + ".knoxshell" + File.separator + fileName), s);
Review comment:
Nice. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] lmccay commented on a change in pull request #241: KNOX-1742
- Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369038160
##########
File path: build-tools/src/main/resources/build-tools/pmd/pmd-ruleset.xml
##########
@@ -88,6 +88,7 @@ limitations under the License.
<rule ref="category/java/multithreading.xml">
<exclude name="AvoidSynchronizedAtMethodLevel" />
<exclude name="UseConcurrentHashMap" />
+ <exclude name="DoNotUseThreads" />
Review comment:
Cool. I'll change it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] lmccay commented on a change in pull request #241: KNOX-1742
- Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369037891
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
##########
@@ -593,21 +594,42 @@ public String toString() {
String s = JsonUtils.renderAsJsonString(map);
String home = System.getProperty("user.home");
try {
- write(new File(home + File.separator + ".knoxshell" + File.separator + fileName), s);
+ write(new File(
+ home + File.separator + ".knoxshell" + File.separator + fileName),
+ s, StandardCharsets.UTF_8);
} catch (IOException e) {
e.printStackTrace();
}
}
- private static void write(File file, String s) throws IOException {
+ /**
+ * Persist provided Map to a file within the {user.home}/.knoxshell directory
+ * @param <T>
+ * @param fileName of persisted file
+ * @param map to persist
+ */
+ public static <T> void persistDataSourcesToKnoxShell(String fileName, Map<String, T> map) {
+ String s = JsonUtils.renderAsJsonString(map);
+ String home = System.getProperty("user.home");
+ try {
+ write(new File(
+ home + File.separator +
+ ".knoxshell" + File.separator + fileName),
+ s, StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ }
+
+ private static void write(File file, String s, Charset utf8) throws IOException {
Review comment:
Ahhh - had a merge conflict with that and thought someone else must have added it. I see now that you removed for the javadoc error patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #241:
KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #241: KNOX-1742 - Simple SQL Client in KnoxShell for access to JDBC sources
URL: https://github.com/apache/knox/pull/241#discussion_r369100671
##########
File path: gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSession.java
##########
@@ -593,7 +593,26 @@ public String toString() {
String s = JsonUtils.renderAsJsonString(map);
String home = System.getProperty("user.home");
try {
- write(new File(home + File.separator + ".knoxshell" + File.separator + fileName), s);
+ write(new File(
+ home + File.separator + ".knoxshell" + File.separator + fileName), s);
Review comment:
Just a note for future reference:
This could be `Paths.get(home, ".knoxshell", fileName).toFile()` or just use the `Path` directly instead of manually constructing a path.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services