You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "turboFei (via GitHub)" <gi...@apache.org> on 2023/06/15 02:11:51 UTC
[GitHub] [kyuubi] turboFei opened a new pull request, #4965: [BEELINE] Remove comments for non python mode
turboFei opened a new pull request, #4965:
URL: https://github.com/apache/kyuubi/pull/4965
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230572592
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
It seems my fault in #4879 . Apologize for adding the unrelated argument in the first place.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230779100
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
#4969 is trying to run Junit tests in the beeline module. Let's see whether it triggers the failures.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1594325484
Could you help to merge this？ I am out of office today.
@pan3793 cc @bowenliang123 as well because the beeline UT is blocked By this.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230779100
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
#4969 is trying to run Junit tests along with ScalaTest. Let's see whether it triggers the failures in beeline module.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592751485
will check whether we need https://github.com/apache/kyuubi/pull/4619 after this pr.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230360180
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -50,6 +56,13 @@ public KyuubiBeeLine(boolean isBeeLine) {
super(isBeeLine);
try {
DynFields.builder().hiddenImpl(BeeLine.class, "commands").buildChecked(this).set(commands);
+
+ Field resourceBundleField = BeeLine.class.getDeclaredField("resourceBundle");
+ resourceBundleField.setAccessible(true);
+ Field modifiers = Field.class.getDeclaredField("modifiers");
+ modifiers.setAccessible(true);
+ modifiers.setInt(resourceBundleField, resourceBundleField.getModifiers() & ~Modifier.FINAL);
Review Comment:
will move this part to DynFields
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230372742
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -50,6 +56,13 @@ public KyuubiBeeLine(boolean isBeeLine) {
super(isBeeLine);
try {
DynFields.builder().hiddenImpl(BeeLine.class, "commands").buildChecked(this).set(commands);
+
+ Field resourceBundleField = BeeLine.class.getDeclaredField("resourceBundle");
+ resourceBundleField.setAccessible(true);
+ Field modifiers = Field.class.getDeclaredField("modifiers");
+ modifiers.setAccessible(true);
+ modifiers.setInt(resourceBundleField, resourceBundleField.getModifiers() & ~Modifier.FINAL);
Review Comment:
According to https://stackoverflow.com/questions/3301635/change-private-static-final-field-using-java-reflection
this is dangerous
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1594197112
Cc @cfmcgrady any other comments？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230992155
##########
kyuubi-hive-beeline/src/test/java/org/apache/hive/beeline/KyuubiBeeLineTest.java:
##########
@@ -47,4 +52,73 @@ public void testKyuubiBeelineExitCodeWithoutConnection() {
int result3 = kyuubiBeeLine.initArgs(args3);
assertEquals(1, result3);
}
+
+ @Test
+ public void testKyuubiBeeLineCmdUsage() {
+ BufferPrintStream printStream = new BufferPrintStream();
+
+ KyuubiBeeLine kyuubiBeeLine = new KyuubiBeeLine();
+ DynFields.builder()
+ .hiddenImpl(BeeLine.class, "outputStream")
+ .build(kyuubiBeeLine)
+ .set(printStream);
+ String[] args1 = {"-h"};
+ kyuubiBeeLine.initArgs(args1);
+ String output = printStream.getOutput();
+ assert output.contains("--python-mode Execute python code/script.");
+ }
+
+ @Test
+ public void testKyuubiBeeLinePythonMode() {
+ KyuubiBeeLine kyuubiBeeLine = new KyuubiBeeLine();
+ String[] args1 = {"-u", "badUrl", "--python-mode"};
+ kyuubiBeeLine.initArgs(args1);
+ assertTrue(kyuubiBeeLine.isPythonMode());
+ kyuubiBeeLine.setPythonMode(false);
+
+ String[] args2 = {"--python-mode", "-f", "test.sql"};
+ kyuubiBeeLine.initArgs(args2);
+ assertTrue(kyuubiBeeLine.isPythonMode());
+ assert kyuubiBeeLine.getOpts().getScriptFile().equals("test.sql");
+ kyuubiBeeLine.setPythonMode(false);
+
+ String[] args3 = {"-u", "badUrl"};
+ kyuubiBeeLine.initArgs(args3);
+ assertTrue(!kyuubiBeeLine.isPythonMode());
+ kyuubiBeeLine.setPythonMode(false);
+ }
+
+ static class BufferPrintStream extends PrintStream {
+ public StringBuilder stringBuilder = new StringBuilder();
+
+ static OutputStream noOpOutputStream =
Review Comment:
seems unused
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230572280
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -248,4 +283,34 @@ int runInit() {
}
return executionResult;
}
+
+ static class KyuubiBeelineResourceBundle extends ListResourceBundle {
+ static String CMD_USAGE = "cmd-usage";
+
+ private Object[][] contents = new Object[beelineResourceBundle.keySet().size()][];
+
+ public KyuubiBeelineResourceBundle() {
+ int i = 0;
+ for (String key : beelineResourceBundle.keySet()) {
+ String value = beelineResourceBundle.getString(key);
+ if (key.equals(CMD_USAGE)) {
+ StringBuilder stringBuilder = new StringBuilder();
+ stringBuilder.append(value).append("\n");
+ stringBuilder
+ .append("Usage: java " + KyuubiBeeLine.class.getCanonicalName())
+ .append("\n");
+ stringBuilder.append(
+ " --python-mode=[true/false] Execute python code/script.");
Review Comment:
done
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1231630324
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiCommands.java:
##########
@@ -44,24 +45,27 @@ public boolean sql(String line) {
return execute(line, false, false);
}
+ /** For python mode, keep it as it is. */
+ private String trimForNonPythonMode(String line) {
+ return beeLine.isPythonMode() ? line : line.trim();
+ }
+
/** Extract and clean up the first command in the input. */
private String getFirstCmd(String cmd, int length) {
- return cmd.substring(length).trim();
+ return trimForNonPythonMode(cmd.substring(length));
}
private String[] tokenizeCmd(String cmd) {
return cmd.split("\\s+");
}
private boolean isSourceCMD(String cmd) {
- cmd = cmd.trim();
if (cmd == null || cmd.isEmpty()) return false;
Review Comment:
FYI:
https://github.com/apache/kyuubi/pull/4965#issuecomment-1592767770
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cfmcgrady commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592688862
Should we consider reverting the removal of the `trim` related operation that was committed in https://github.com/apache/kyuubi/pull/3762? It's dangerous for non-python mode.
```diff
diff --git a/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiCommands.java b/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiCommands.java
index e3a4b295e..3ffe7aee7 100644
--- a/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiCommands.java
+++ b/kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiCommands.java
@@ -21,7 +21,6 @@ import java.io.*;
import java.sql.*;
import java.util.*;
import org.apache.hive.beeline.logs.KyuubiBeelineInPlaceUpdateStream;
-import org.apache.hive.common.util.HiveStringUtils;
import org.apache.kyuubi.jdbc.hive.JdbcConnectionParams;
import org.apache.kyuubi.jdbc.hive.KyuubiStatement;
import org.apache.kyuubi.jdbc.hive.Utils;
@@ -45,7 +44,7 @@ public class KyuubiCommands extends Commands {
/** Extract and clean up the first command in the input. */
private String getFirstCmd(String cmd, int length) {
- return cmd.substring(length).trim();
+ return cmd.substring(length);
}
private String[] tokenizeCmd(String cmd) {
@@ -97,7 +96,6 @@ public class KyuubiCommands extends Commands {
}
String[] cmds = lines.split(";");
for (String c : cmds) {
- c = c.trim();
if (!executeInternal(c, false)) {
return false;
}
@@ -261,10 +259,9 @@ public class KyuubiCommands extends Commands {
beeLine.handleException(e);
}
- line = line.trim();
List<String> cmdList = getCmdList(line, entireLineAsCommand);
for (int i = 0; i < cmdList.size(); i++) {
- String sql = cmdList.get(i).trim();
+ String sql = cmdList.get(i);
if (sql.length() != 0) {
if (!executeInternal(sql, call)) {
return false;
@@ -511,7 +508,6 @@ public class KyuubiCommands extends Commands {
@Override
public String handleMultiLineCmd(String line) throws IOException {
int[] startQuote = {-1};
- line = HiveStringUtils.removeComments(line, startQuote);
Character mask =
(System.getProperty("jline.terminal", "").equals("jline.UnsupportedTerminal"))
? null
@@ -542,7 +538,6 @@ public class KyuubiCommands extends Commands {
if (extra == null) { // it happens when using -f and the line of cmds does not end with ;
break;
}
- extra = HiveStringUtils.removeComments(extra, startQuote);
if (!extra.isEmpty()) {
line += "\n" + extra;
}
@@ -554,13 +549,12 @@ public class KyuubiCommands extends Commands {
// console. Used in handleMultiLineCmd method assumes line would never be null when this method is
// called
private boolean isMultiLine(String line) {
- line = line.trim();
if (line.endsWith(beeLine.getOpts().getDelimiter()) || beeLine.isComment(line)) {
return false;
}
// handles the case like line = show tables; --test comment
List<String> cmds = getCmdList(line, false);
- return cmds.isEmpty() || !cmds.get(cmds.size() - 1).trim().startsWith("--");
+ return cmds.isEmpty() || !cmds.get(cmds.size() - 1).startsWith("--");
}
static class KyuubiLogRunnable implements Runnable {
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230568054
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
cc @bowenliang123
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1231626443
##########
kyuubi-hive-beeline/src/test/java/org/apache/hive/beeline/KyuubiBeeLineTest.java:
##########
@@ -47,4 +52,73 @@ public void testKyuubiBeelineExitCodeWithoutConnection() {
int result3 = kyuubiBeeLine.initArgs(args3);
assertEquals(1, result3);
}
+
+ @Test
+ public void testKyuubiBeeLineCmdUsage() {
+ BufferPrintStream printStream = new BufferPrintStream();
+
+ KyuubiBeeLine kyuubiBeeLine = new KyuubiBeeLine();
+ DynFields.builder()
+ .hiddenImpl(BeeLine.class, "outputStream")
+ .build(kyuubiBeeLine)
+ .set(printStream);
+ String[] args1 = {"-h"};
+ kyuubiBeeLine.initArgs(args1);
+ String output = printStream.getOutput();
+ assert output.contains("--python-mode Execute python code/script.");
+ }
+
+ @Test
+ public void testKyuubiBeeLinePythonMode() {
+ KyuubiBeeLine kyuubiBeeLine = new KyuubiBeeLine();
+ String[] args1 = {"-u", "badUrl", "--python-mode"};
+ kyuubiBeeLine.initArgs(args1);
+ assertTrue(kyuubiBeeLine.isPythonMode());
+ kyuubiBeeLine.setPythonMode(false);
+
+ String[] args2 = {"--python-mode", "-f", "test.sql"};
+ kyuubiBeeLine.initArgs(args2);
+ assertTrue(kyuubiBeeLine.isPythonMode());
+ assert kyuubiBeeLine.getOpts().getScriptFile().equals("test.sql");
+ kyuubiBeeLine.setPythonMode(false);
+
+ String[] args3 = {"-u", "badUrl"};
+ kyuubiBeeLine.initArgs(args3);
+ assertTrue(!kyuubiBeeLine.isPythonMode());
+ kyuubiBeeLine.setPythonMode(false);
+ }
+
+ static class BufferPrintStream extends PrintStream {
+ public StringBuilder stringBuilder = new StringBuilder();
+
+ static OutputStream noOpOutputStream =
Review Comment:
it is used.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei closed pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
URL: https://github.com/apache/kyuubi/pull/4965
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1637131603
backported to 1.7 via 36ddb0db15c17275295ed58ee61a409f720205cc
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230576190
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
could you help check why the GA pass and did not detect this issue?
It helps a lot to prevent it happens again.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cxzl25 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230365693
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -248,4 +283,34 @@ int runInit() {
}
return executionResult;
}
+
+ static class KyuubiBeelineResourceBundle extends ListResourceBundle {
+ static String CMD_USAGE = "cmd-usage";
+
+ private Object[][] contents = new Object[beelineResourceBundle.keySet().size()][];
+
+ public KyuubiBeelineResourceBundle() {
+ int i = 0;
+ for (String key : beelineResourceBundle.keySet()) {
+ String value = beelineResourceBundle.getString(key);
+ if (key.equals(CMD_USAGE)) {
+ StringBuilder stringBuilder = new StringBuilder();
+ stringBuilder.append(value).append("\n");
+ stringBuilder
+ .append("Usage: java " + KyuubiBeeLine.class.getCanonicalName())
+ .append("\n");
+ stringBuilder.append(
+ " --python-mode=[true/false] Execute python code/script.");
Review Comment:
Maybe add tips to the migration guide `docs/deployment/migration-guide.md`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230348989
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -50,6 +56,13 @@ public KyuubiBeeLine(boolean isBeeLine) {
super(isBeeLine);
try {
DynFields.builder().hiddenImpl(BeeLine.class, "commands").buildChecked(this).set(commands);
+
+ Field resourceBundleField = BeeLine.class.getDeclaredField("resourceBundle");
+ resourceBundleField.setAccessible(true);
+ Field modifiers = Field.class.getDeclaredField("modifiers");
+ modifiers.setAccessible(true);
+ modifiers.setInt(resourceBundleField, resourceBundleField.getModifiers() & ~Modifier.FINAL);
Review Comment:
I do not use DynFields because now DynFields can not set final static field.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230779100
##########
kyuubi-hive-beeline/src/main/java/org/apache/hive/beeline/KyuubiBeeLine.java:
##########
@@ -155,7 +182,7 @@ int initArgs(String[] args) {
DynMethods.builder("defaultBeelineConnect")
.hiddenImpl(BeeLine.class, CommandLine.class)
.buildChecked(this)
- .invoke(beelineParser, cl);
+ .invoke(cl);
Review Comment:
#4969 is trying to run Junit tests along with ScalaTest. Let's see whether it triggers the failures in beeline.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] codecov-commenter commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592362918
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/4965?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#4965](https://app.codecov.io/gh/apache/kyuubi/pull/4965?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5d58ada) into [master](https://app.codecov.io/gh/apache/kyuubi/commit/01320c4c2f52ca7183e44e7803b8356df11baf00?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (01320c4) will **not change** coverage.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #4965 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 563 563
Lines 30997 31019 +22
Branches 4044 4048 +4
======================================
- Misses 30997 31019 +22
```
| [Impacted Files](https://app.codecov.io/gh/apache/kyuubi/pull/4965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...in/java/org/apache/hive/beeline/KyuubiBeeLine.java](https://app.codecov.io/gh/apache/kyuubi/pull/4965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWhpdmUtYmVlbGluZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGl2ZS9iZWVsaW5lL0t5dXViaUJlZUxpbmUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...n/java/org/apache/hive/beeline/KyuubiCommands.java](https://app.codecov.io/gh/apache/kyuubi/pull/4965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLWhpdmUtYmVlbGluZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGl2ZS9iZWVsaW5lL0t5dXViaUNvbW1hbmRzLmphdmE=) | `0.00% <0.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592592593
@cfmcgrady how do you think this way?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cxzl25 commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1593137948
> Should we consider reverting the removal of the `trim` related operation that was committed in #3762? It's dangerous for non-python mode.
Now add the trim method, but if trim is still called in python mode, will it affect the python script execution?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592767770
> #4619
not needed,
both these two methods are invoked in `executeInternal` only.
and before invoking executeInternal, the line is trimed. so we can revert it to keep align with upstream.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592743787
@cfmcgrady I checked with upstream.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#discussion_r1230999942
##########
docs/deployment/migration-guide.md:
##########
@@ -24,6 +24,10 @@
To restore previous behavior, set `kyuubi.metadata.store.jdbc.database.type=DERBY` and
`kyuubi.metadata.store.jdbc.url=jdbc:derby:memory:kyuubi_state_store_db;create=true`.
+## Upgrading from Kyuubi 1.7.1 to 1.7.2
+
+* Since Kyuubi 1.7.2, for Kyuubi BeeLine, please use `--python-mode` option to run python code or script.
Review Comment:
Minor: Maybe worth to mention this option in the description.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1593849119
> > Should we consider reverting the removal of the `trim` related operation that was committed in #3762? It's dangerous for non-python mode.
>
> Now add the trim method, but if trim is still called in python mode, will it affect the python script execution?
@cxzl25 done, could you help check it again?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] cfmcgrady commented on pull request #4965: [BEELINE] Remove comments for non python mode
Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1592660261
> @cfmcgrady how do you think this way?
As we discussed in https://github.com/apache/kyuubi/issues/4803#issuecomment-1578366167, I think this is a good approach.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] turboFei commented on pull request #4965: [BEELINE] Support `--python-mode` option and remove comments for non-python mode
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4965:
URL: https://github.com/apache/kyuubi/pull/4965#issuecomment-1594401707
thanks, merged to master first
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org