-
Notifications
You must be signed in to change notification settings - Fork 8
Make QueryEditor as a reusable react component #21
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
Conversation
b102d69 to
1e23033
Compare
mosabua
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First review round done .. I have to test it some more and see what you think about my input .. beyond that I think we should merge it soon so we can see if it works and then follow up with more refinements after a first release
.github/workflows/release.yml
Outdated
| release: | ||
| types: [created] | ||
| pull_request: | ||
| types: [published] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change - compare to js client - https://github.com/trinodb/trino-js-client/blob/4a7a97298dca27cc080cd85544a75d9185c07d23/.github/workflows/release.yml#L4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn’t make a difference but according to the docs the published event seemed more accurate than created. I’ve updated the PR though to use created so the gh workflows are now consistent across both projects.
Docs:
types: [created] → fires when a release object is first created (often as a draft). But GitHub does not trigger workflows for created (or edited/deleted) if the release is a draft created via the UI.
GitHub Docs
types: [published] → fires when a release (or pre-release) is published (made visible). This is the most common choice for CI/CD on new releases."_
README.md
Outdated
| # Trino Query UI | ||
|
|
||
| This Trino Query UI is a new UI component that can be integrated into Trino and run directly from the Trino installation at the `/query/` path. For testing, it can also be run separately and can proxy to a Trino running locally or elsewhere. | ||
| A reusable React component for executing queries against Trino. It can be embedded into any react application and configured to proxy requests to a local or remote Trino cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| A reusable React component for executing queries against Trino. It can be embedded into any react application and configured to proxy requests to a local or remote Trino cluster. | |
| A reusable React component for executing queries against Trino. It can be embedded into any React application and configured to proxy requests to a local or remote Trino cluster. |
also please wrap at 80 column width here and other paragraphs
README.md
Outdated
| A reusable React component for executing queries against Trino. It can be embedded into any react application and configured to proxy requests to a local or remote Trino cluster. | ||
|
|
||
| > [!WARNING] | ||
| > This package is under heavy development and is not yet recommended for production workloads. Treat the current release as an early-stage demo; production-ready builds and documentation will follow soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > This package is under heavy development and is not yet recommended for production workloads. Treat the current release as an early-stage demo; production-ready builds and documentation will follow soon. | |
| > This package is under heavy development and is not yet recommended for production workloads. Treat the current release as an early-stage demo; production-ready builds and documentation are planned. |
README.md
Outdated
| return false; | ||
| } | ||
| } | ||
| ## Quick Start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Quick Start | |
| ## Quick start |
README.md
Outdated
|
|
||
| The local URL will be be displayed which you can open in your browser. | ||
|
|
||
| ### Building the Parser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### Building the Parser | |
| ### Building the parser |
README.md
Outdated
| Run `npm run antlr4ng` to build the parser, as configured in **package.json**. | ||
|
|
||
| ### Linting and code formatting | ||
| ### Linting and Code Formatting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### Linting and Code Formatting | |
| ### Linting and code formatting |
README.md
Outdated
| - Show stages and split counts like the Trino console client | ||
| 4. Keep the experience easy to navigate | ||
|
|
||
| ### Gaps and Future Direction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### Gaps and Future Direction | |
| ### Gaps and future direction |
precise/package.json
Outdated
| "type": "git", | ||
| "url": "https://github.com/trinodb/trino-query-ui.git" | ||
| }, | ||
| "homepage": "https://trinodb.github.io/trino-query-ui", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont have that set up at this stage so lets use the same URL as for the source
|
Also note the npm token should be available as it set organization-wide now .. |
ae8c126 to
5b34232
Compare
5b34232 to
d032be6
Compare
|
@mosabua thanks I addressed all comments. let me know if I missed something |
mosabua
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor comments that can be addressed in a follow up PR. I am going to merge this in the interest of testing the release flow next. We should start with some 0.x version but move to 1.x soon since the component is already quite mature and powerfule. cc @georgewfisher
| <link rel="icon" type="image/png" href="/commander_bunbun.png" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Trino Query Editor</title> | ||
| <title>Trino Query Editor - Example app</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <title>Trino Query Editor - Example app</title> | |
| <title>Trino query editor - Example app</title> |
| "private": true, | ||
| "version": "0.0.0", | ||
| "name": "trino-query-ui", | ||
| "description": "Trino Query Editor react component", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "description": "Trino Query Editor react component", | |
| "description": "Trino query editor React component", |
|
|
||
| async parseAndDecoratePromiseAsync(monaco: any, editor: any, waitForUserToStopTyping: number): Promise<boolean> { | ||
| if (this.isRunningParse) { | ||
| //console.error("cancelled parsing:" + this.isRunningParse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now we can leave this I guess but over time we should either make it configurable somehow or remove.
| ReactDOM.createRoot(document.getElementById('root')!).render(<App />) | ||
| ReactDOM.createRoot(document.getElementById('root')!).render( | ||
| <React.StrictMode> | ||
| <h1>Trino Query Editor - Example app</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <h1>Trino Query Editor - Example app</h1> | |
| <h1>Trino query editor - Example app</h1> |
| } | ||
|
|
||
| handleTabCreate = () => { | ||
| const newQuery = this.props.queries.addQuery(false, 'New Query') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const newQuery = this.props.queries.addQuery(false, 'New Query') | |
| const newQuery = this.props.queries.addQuery(false, 'New query') |
Description
This PR introduces the initial version of the trino
<QueryEditor>reusable react component primarily to enable the publishing workflow as an npm package. Once merged the GHAreleaseworkflow should publish the first version of the package to npm.IMPORTANT:
NPM_TOKENgithub secret must be set in this github repo!The component is already a functional Trino web query runner. However, it currently lacks configurable parameters, styling, and requires several further major refinements. These improvements will be addressed separately and incrementally after confirming that the npm publishing process works as expected. This initial version (
0.0.1) is not yet recommended for production workloads. Production-ready builds and documentation will follow soon.Example usage:
it can be used in any react project (such as the main trino or the trino-gateway UI):
Additional context and related issues