Skip to content
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.

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

Cluster Manager UI & Query Console UI revamp #5684

Merged
merged 8 commits into from
Jul 17, 2020
Merged

Cluster Manager UI & Query Console UI revamp #5684

merged 8 commits into from
Jul 17, 2020

Conversation

shahsank3t
Copy link
Contributor

Description

Complete revamp of existing UI containing Cluster Manager & Query Console.

  • This PR is against Cluster Manager  #5649

  • Cluster Manager UI contains a listing of Tenants & Table within, Broker, Controller, Server, Cluster Configuration
    screencapture-localhost-9000-2020-07-11-02_30_04
    screencapture-localhost-9000-2020-07-11-02_33_16

  • Query Console UI is a complete revamp with Material UI styling.
    screencapture-localhost-9000-2020-07-11-02_35_51

  • This PR removes all the existing pinot-controller's static & webapp and gets replaced with an app developed using React + TypeScript + Material UI and few other 3rd Party libraries and built using webpack.

  • Also added a plugin in pinot-controller pom file to build the frontend using node & npm.

Open items that can be added as part of another PR:

  • SQL Hints in SQL Editor
  • Reporting error when SQL Query fails

@mayankshriv
Copy link
Contributor

A couple suggestions:

  • For cluster manager, we should be able to drill down at table level as well (apart from tenant).
  • It would be super awesome if the UI provides a way to authenticate for certain operations (such as query, write operations on cluster like schema update etc). Note, we already have ACL support in the code, we just need the UI to pass the credentials.

@kishoreg
Copy link
Member

@mayankshriv this is the first cut. lets get this in and we can add more functionality in the subsequent PR

@shahsank3t
Copy link
Contributor Author

testQuery Test was failing but we longer have static page for query and so removed it.

@kishoreg
Copy link
Member

kishoreg commented Jul 12, 2020

@shahsank3t This looks great! I tried this locally and it worked seamlessly. I was also able to bring it up by running quickstart from IDE (though it requires one to build the project from the command line)

query editor has some minor issues. The cursor keeps jumping/moving to the end as I type. This happens when you are trying to edit an existing query. typing new query works fine

@mayankshriv
Copy link
Contributor

@mayankshriv this is the first cut. lets get this in and we can add more functionality in the subsequent PR

@kishoreg Yes agree, my comment is more towards the general direction of where we should take it. Also, I take it that cluster manager UI will replace Swagger? Would be good to start a doc/issue to capture all the features we would want cluster manager to have and address them in future PRs.

@snleee
Copy link
Contributor

snleee commented Jul 13, 2020

@shahsank3t I see that you are deleting a lot of js files. I think that the license file needs to be updated accordingly. Can you fix LICENSE file? You will see the following lines:


MIT License
-----------
pinot-controller/src/main/resources/*/js/lib/codemirror/*
pinot-controller/src/main/resources/*/js/lib/foundation/*
pinot-controller/src/main/resources/*/js/lib/angular*
pinot-controller/src/main/resources/*/js/lib/underscore*
pinot-controller/src/main/resources/*/js/lib/jquery-2.1.3.min.js
pinot-controller/src/main/resources/*/js/lib/beautify.js
pinot-controller/src/main/resources/*/js/lib/handlebars.js
pinot-controller/src/main/resources/*/css/lib/codemirror*
pinot-controller/src/main/resources/*/css/lib/foundation*
pinot-controller/src/main/resources/*/css/lib/normalize.css

BSD 3-Clause
------------
pinot-controller/src/main/resources/*/js/lib/jquery.dataTables.min.js

@shahsank3t
Copy link
Contributor Author

@kishoreg
Good Catch. Fixed the SQL code editor cursor issue.

@snleee
Updated the existing LICENSE file with the dependencies & dev-dependencies libraries license info. Please have a look and let me know if need to add any more info. Thanks in advance.

@shahsank3t
Copy link
Contributor Author

  • For cluster manager, we should be able to drill down at table level as well (apart from the tenant).

@kishoreg @mayankshriv - Addressed this suggestion and added the Tables details page. Yes, currently showing the JSON object and later on can add incremental updates to it.
screencapture-localhost-8080-2020-07-15-20_18_50

@kishoreg kishoreg merged commit ffbc8c3 into apache:master Jul 17, 2020
@shahsank3t shahsank3t deleted the cluster-query-ui branch July 21, 2020 04:05
snleee pushed a commit to snleee/pinot that referenced this pull request May 19, 2021
The major code change in the UI(apache#5684) removed all the bundled
javascript/css files. This change removed bunch of copied js/css files
but the pr did not correctly address the license.

This PR is the effort for cleaning up licenses.

1. Cleaned up LICENSE, LICENSE-binary
2. Cleaned up licenses/, licenses-binary/
3. Cleaned up NOTICE-binary
@snleee snleee mentioned this pull request May 19, 2021
snleee pushed a commit that referenced this pull request May 24, 2021
The major code change in the UI(#5684) removed all the bundled
javascript/css files. This change removed bunch of copied js/css files
but the pr did not correctly address the license.

This PR is the effort for cleaning up licenses.

1. Cleaned up LICENSE, LICENSE-binary
2. Cleaned up licenses/, licenses-binary/
3. Cleaned up NOTICE-binary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants