[KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI #5616

zhaohehuhu · 2023-11-03T07:37:28Z

Why are the changes needed?

This PR is to introduce a submit SQL editor page to close #5366. Users then can submit SQL in SQL editor page to retrieve, update or do other data manipulations by Spark engine with this feature.

Once users open up a new tab in SQL editor, a new connection(session) is established with the a Kyuubi server instance. After that, users can input their SQL statements in a text box provided on the page and select the desired number of data rows to be shown in result from a dropdown list located at the right side of the run button. When the statement is executed in one of server instances, the execution status of the job is displayed to the user. After the statement is executed successfully , the result is displayed in the bottom of page for users.

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 locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No

(cherry picked from commit c4c2b6c)

(cherry picked from commit 1fac99a)

(cherry picked from commit 902d01d)

(cherry picked from commit be54d44)

(cherry picked from commit 6f0d5f8)

(cherry picked from commit 521facb)

(cherry picked from commit d45df20)

zwangsheng · 2023-11-03T07:50:18Z

kyuubi-server/web-ui/src/components/monaco-editor/type.ts

+    type: String as PropType<any>,
+    // validator(value: string): boolean {
+    //   return ['vs', 'vs-dark'].includes(value)
+    // },


This comment doesn't seem to make sense, maybe we can remove it?

zwangsheng · 2023-11-03T07:52:45Z

kyuubi-server/web-ui/src/api/editor/index.ts

+
+import request from '@/utils/request'
+
+export function openSession(data: any): any {


Need to follow TypeScript strict mode and avoid heavy use of any.

Can we wrap a class to use?

zwangsheng · 2023-11-03T07:53:33Z

kyuubi-server/web-ui/src/api/server/index.ts

@@ -17,7 +17,7 @@

 import request from '@/utils/request'

-export function getAllServer() {
+export function getAllServer(): any {


Return Array<IServer>?

zwangsheng · 2023-11-03T07:54:40Z

kyuubi-server/web-ui/src/router/index.ts

@@ -20,7 +20,7 @@ import overviewRoutes from './overview'
 import managementRoutes from './management'
 import detailRoutes from './detail'
 import swaggerRoutes from './swagger'
-import labRoutes from './lab'
+import labRoutes from './editor'


labRoutes => editorRoutes

zwangsheng · 2023-11-03T08:02:46Z

kyuubi-server/web-ui/src/views/editor/components/Editor.vue

+<template>
+  <div class="editor">
+    <el-space>
+      <el-select v-model="param.engineType" :placeholder="$t('engine_type')">


We only support Spark SQL Engine now, so maybe we should disable this el-select.

zwangsheng · 2023-11-03T08:09:02Z

kyuubi-server/web-ui/src/views/editor/components/Editor.vue

+    engineType: 'SPARK_SQL'
+  })
+  const limit = ref(10)
+  const limits = ref([10, 50, 100])


We don't need to bind this limits with two-way data binding, simple constants should suffice.

zwangsheng · 2023-11-03T08:16:38Z

kyuubi-server/web-ui/src/views/editor/components/Editor.vue

+      .then((res: ILog) => {
+        return res?.logRowSet?.join('\r\n')
+      })
+      .catch(() => '')


Should catch error

zwangsheng · 2023-11-03T08:19:05Z

kyuubi-server/web-ui/src/views/editor/components/Editor.vue

+        })
+      })
+      .catch(() => {
+        sqlResult.value = []


Should catch error

zwangsheng · 2023-11-03T08:21:25Z

kyuubi-server/web-ui/src/locales/en_US/index.ts

@@ -37,21 +37,25 @@ export default {
  engine_ui: 'Engine UI',
  failure_reason: 'Failure Reason',
  session_properties: 'Session Properties',
+  no_data: 'No data yet',


Please correct me if this is incorrect, we return no_data when query success with empty data, so this should be 'No data'?

fix

codecov-commenter · 2023-11-06T03:05:58Z

Codecov Report

Merging #5616 (078d19e) into master (04cae6a) will increase coverage by 0.02%.
Report is 19 commits behind head on master.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #5616      +/-   ##
============================================
+ Coverage     61.38%   61.41%   +0.02%     
  Complexity       23       23              
============================================
  Files           603      603              
  Lines         35635    35664      +29     
  Branches       4874     4876       +2     
============================================
+ Hits          21874    21902      +28     
- Misses        11381    11383       +2     
+ Partials       2380     2379       -1

see 28 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

zwangsheng

LGTM with little comments, also PTAL @pan3793

zwangsheng · 2023-11-06T03:26:11Z

kyuubi-server/web-ui/src/views/editor/components/Editor.vue

+        })
+      })
+      .catch(() => {
+        ElMessage({


Use errorCatch instead

Hi, thanks for review. errorCatch has its own logic like set sqlResult and sqlLog to empty, not very suitable to use it here.

Thanks for your comment. In my opinion, when we run this SQL and encounter an error, we already have both set the result to an empty array and used the el-message component to notify the user. However, should we also try to obtain the SQL log from the error message if it is available?

@zwangsheng
Certainly, got your point, what you think is the api error message might be helpful to users, we also need it!

If we want to display all the error info for the APIs through the log, there might be some issues here:
The current design for executing SQL is as follows:

First, we call the openSession API, which returns the sessionIdentifier.

Then insert the returned sessionIdentifier into the second API, runSql, which returns the runSqlIdentifier.
These two steps must be executed synchronously because they are dependent on each other.

After the above steps, there are two more steps remaining:

Retrieving the SQL's return results (here, there are two APIs, one for getting the table's field names, and one for getting the table's field values).

Getting the SQL log.
Here, considering the user experience, we don't need to make the APIs execute synchronously. That is, getting the SQL results and getting the SQL log will be sent asynchronously to the service, as the SQL log API doesn't need to wait for the SQL result, and the SQL result API doesn't need to wait for the SQL log.

Now, here's the issue:
Because the last three APIs are asynchronous, if we only overwrite their error information into the log, it might provide a poor user experience. For example, suppose the getSqlResult API returns an error in 30 seconds (it could be two errors), and it logs the error, then user is watching the log, but suddenly the log gets overwritten because at 45 seconds, the getSqlLog also sends logs. Appending the log to the API isn't good enough either because the log display might not be very clear.

If this error log is useful for user, can we consider this design below?

The log section will only display query log information.

Error messages will be placed in the error box so that users can clearly see the error content and list error messages from multiple APIs as a reference. (see screenshot)

Thanks.

Hi @zwangsheng
Has added new error messages for single or multiple errors, please help review, thanks!

@tongwl could you put the Run button at the head of the line? and use a light color for it?

Also, please use SQL instead of Sql in the message.

Hi @pan3793 , sure, got!

add error message for sql page

* [KYUUBI apache#5626] [UI] Fix sytle linting violation in web ui ### _Why are the changes needed?_ - To fix the style violation in web UI linting `cd ./kyuubi-server/web-ui && pnpm run lint` ``` > kyuubi-ui1.9.0-SNAPSHOT lint /Users/bw/dev/kyuubi/kyuubi-server/web-ui > eslint --ext .ts,vue --ignore-path .gitignore . /Users/bw/dev/kyuubi/kyuubi-server/web-ui/src/test/unit/views/layout/aside.spec.ts 37:25 error Delete `⏎····` prettier/prettier ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option. ELIFECYCLE Command failed with exit code 1. ``` ### _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 - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes apache#5626 from bowenliang123/webui-lint-fix. Closes apache#5626 b1c7d7b [Bowen Liang] web ui lint fix Authored-by: Bowen Liang <liangbowen@gf.com.cn> Signed-off-by: Cheng Pan <chengpan@apache.org> * [KYUUBI apache#5579][AUTHZ] Support LogicalRelation don't have CatalogTable but have HadoopFsRelation ### _Why are the changes needed?_ To close apache#5579 Support LogicalRelation don't have CatalogTable but have HadoopFsRelation ### _How was this patch tested?_ - [x] 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/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes apache#5581 from AngersZhuuuu/KYUUBI-5579. Closes apache#5579 0298db5 [Angerszhuuuu] Update uriExtractors.scala 05a9480 [Angerszhuuuu] update 5acc919 [Angerszhuuuu] Update PrivilegesBuilder.scala 77cc7f9 [Angerszhuuuu] update 47b79c7 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala 96f2006 [Angerszhuuuu] Merge branch 'master' into KYUUBI-5579 651f3f6 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala 8b5a650 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala c37d655 [Angerszhuuuu] Merge branch 'master' into KYUUBI-5579 a71f3a6 [Angerszhuuuu] update d4bb5b4 [Angerszhuuuu] Update RangerSparkExtensionSuite.scala 6f634f4 [Angerszhuuuu] Merge branch 'master' into KYUUBI-5579 33282e2 [Angerszhuuuu] [KYUUBI apache#5579][AUTHZ] Support LogicalRelation don't have CatalogTable but have HadoopFsRelation Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> * [KYUUBI apache#5533][AUTHZ] Support merge into table command for Delta Lake ### _Why are the changes needed?_ To close apache#5533 . Support merge into table command for Delta Lake. https://docs.delta.io/latest/delta-update.html#upsert-into-a-table-using-merge ### _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 - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No. Closes apache#5621 from zml1206/KYUUBI-5533. Closes apache#5533 71af24a [zml1206] Support merge into table command for Delta Lake Authored-by: zml1206 <zhuml1206@gmail.com> Signed-off-by: Bowen Liang <liangbowen@gf.com.cn> * [KYUUBI apache#5630][Authz] Support path check of LoadDataCommand ### _Why are the changes needed?_ To close apache#5630 Support path check of LoadDataCommand ### _How was this patch tested?_ - [x] 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/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes apache#5632 from AngersZhuuuu/KYUUBi-5630. Closes apache#5630 885a1d7 [Angerszhuuuu] [KYUUBI apache#5630][Authz] Support path check of LoadDataCommand Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org> * [KYUUBI apache#5634] [UI] Smoother the icon animation on collapsing sidebar ### _Why are the changes needed?_ * Renamed type.ts to types.ts as the project uses the naming convention "types.ts" uniformly. * Removed the title 'Swagger' from the Swagger page. * Optimizations for Kyuubi icon display when collapsing the sidebar: make the icon and version jump smoother when expanded. ### _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 <img width="312" alt="image" src="https://github.com/apache/kyuubi/assets/13530192/cbe2b502-a47e-471b-ab77-dab822f793da"> <img width="1731" alt="image" src="https://github.com/apache/kyuubi/assets/13530192/61f0ed56-9328-4046-89aa-bdd1ef6f78e6"> <img width="807" alt="image" src="https://github.com/apache/kyuubi/assets/13530192/cb53a342-ce29-4181-aad5-d32bd32f9009"> - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ Closes apache#5634 from tongwl/master-20231106-2. Closes apache#5634 d2745e3 [weitong] code change 5ac8ce3 [weitong] UI optimize Lead-authored-by: William Tong <weitong@cisco.com> Co-authored-by: weitong <weitong@cisco.com> Signed-off-by: Cheng Pan <chengpan@apache.org> * [KYUUBI apache#5624] [UI] Remove npm run build warning about NODE_ENV=production ### _Why are the changes needed?_ Before this PR, when we run `npm run build` to build production dist will meet: ```log NODE_ENV=production is not supported in the .env file. Only NODE_ENV=development is supported to create a development build of your project. If you need to set process.env.NODE_ENV, you can set it in the Vite config instead. ``` So, let's remove `NODE_ENV=production` in `.env.production`, it's non-used. ### _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 - [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes apache#5624 from zwangsheng/ui/remove_build_warning. Closes apache#5624 20c21fc [zwangsheng] [UI] Remove npm run build warning about NODE_ENV=production Authored-by: zwangsheng <binjieyang@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> --------- Signed-off-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Bowen Liang <liangbowen@gf.com.cn> Co-authored-by: Bowen Liang <liangbowen@gf.com.cn> Co-authored-by: Angerszhuuuu <angers.zhu@gmail.com> Co-authored-by: zml1206 <zhuml1206@gmail.com> Co-authored-by: zwangsheng <binjieyang@apache.org>

zwangsheng

LGTM, cc @pan3793

pan3793 · 2023-11-13T12:17:05Z

Thanks, merged to master

tongwl and others added 8 commits November 3, 2023 15:13

sql page test

9f80e20

(cherry picked from commit c4c2b6c)

add license

dda47bd

(cherry picked from commit 1fac99a)

sql lab page

0ddc6d5

(cherry picked from commit 902d01d)

code change

7379c4e

(cherry picked from commit be54d44)

change layout

159c35a

(cherry picked from commit 6f0d5f8)

move logo picture to images folder

5f4bca5

(cherry picked from commit 521facb)

change format for table header

ab0661f

(cherry picked from commit d45df20)

remove api2

466a5ba

github-actions bot added the module:ui label Nov 3, 2023

zhaohehuhu mentioned this pull request Nov 3, 2023

Add submit SQL page in Kyuubi Web UI #5612

Closed

3 tasks

change TabPaneName to TabPanelName

531761d

zwangsheng reviewed Nov 3, 2023

View reviewed changes

zhaohehuhu and others added 3 commits November 3, 2023 17:24

fix

ecdbe5c

fix

f0669ba

Merge pull request #2 from zhaohehuhu/dev-1103-2

93fa6a2

fix

Merge branch 'master' into dev-1103

f04acf2

zwangsheng approved these changes Nov 6, 2023

View reviewed changes

tongwl and others added 3 commits November 7, 2023 12:55

add error message for sql page

a374028

Merge pull request #3 from zhaohehuhu/dev-1103-3

3438012

add error message for sql page

github-actions bot added module:spark module:extensions module:authz labels Nov 7, 2023

Merge branch 'master' into dev-1103

4bb22f0

github-actions bot removed module:spark module:extensions module:authz labels Nov 7, 2023

zwangsheng approved these changes Nov 7, 2023

View reviewed changes

zwangsheng changed the title ~~Add submit SQL page in Kyuubi Web UI~~ [UI] Add submit SQL page in Kyuubi Web UI Nov 7, 2023

tongwl added 3 commits November 10, 2023 15:11

sql UI change (#5)

265f02d

UI change (#6)

f1fc9a2

fix code conflict in layout (#7)

ddffd19

pan3793 changed the title ~~[UI] Add submit SQL page in Kyuubi Web UI~~ [KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI Nov 10, 2023

tongwl added 2 commits November 13, 2023 13:17

code change (#8)

1d839bc

code optimize (#9)

078d19e

pan3793 approved these changes Nov 13, 2023

View reviewed changes

pan3793 assigned zhaohehuhu Nov 13, 2023

pan3793 added this to the v1.9.0 milestone Nov 13, 2023

pan3793 closed this in 71b099f Nov 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI #5616

[KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI #5616

zhaohehuhu commented Nov 3, 2023 •

edited by pan3793

Loading

zwangsheng Nov 3, 2023

zhaohehuhu Nov 3, 2023

zwangsheng Nov 3, 2023 •

edited

Loading

zhaohehuhu Nov 3, 2023

zwangsheng Nov 3, 2023

zwangsheng Nov 3, 2023

zhaohehuhu Nov 3, 2023

zwangsheng Nov 3, 2023

zwangsheng Nov 3, 2023 •

edited

Loading

zwangsheng Nov 3, 2023

zwangsheng Nov 3, 2023

zwangsheng Nov 3, 2023 •

edited

Loading

codecov-commenter commented Nov 6, 2023 •

edited

Loading

zwangsheng left a comment

zwangsheng Nov 6, 2023

tongwl Nov 6, 2023

zwangsheng Nov 6, 2023

tongwl Nov 6, 2023

tongwl Nov 6, 2023

tongwl Nov 7, 2023

pan3793 Nov 10, 2023

pan3793 Nov 10, 2023

tongwl Nov 10, 2023

tongwl Nov 10, 2023

zwangsheng left a comment

pan3793 commented Nov 13, 2023


		import request from '@/utils/request'

		export function openSession(data: any): any {

[KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI #5616

[KYUUBI #5366][UI] Add submit SQL page in Kyuubi Web UI #5616

Conversation

zhaohehuhu commented Nov 3, 2023 • edited by pan3793 Loading

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwangsheng Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwangsheng Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwangsheng Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Nov 6, 2023 • edited Loading

Codecov Report

zwangsheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwangsheng left a comment

Choose a reason for hiding this comment

pan3793 commented Nov 13, 2023

zhaohehuhu commented Nov 3, 2023 •

edited by pan3793

Loading

zwangsheng Nov 3, 2023 •

edited

Loading

zwangsheng Nov 3, 2023 •

edited

Loading

zwangsheng Nov 3, 2023 •

edited

Loading

codecov-commenter commented Nov 6, 2023 •

edited

Loading