You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/12/08 06:56:27 UTC
[GitHub] [druid] vogievetsky opened a new pull request #12037: Web console: use query actions in query view
vogievetsky opened a new pull request #12037:
URL: https://github.com/apache/druid/pull/12037
Right now there is a bug that can lead to parts of queries typed into the query view being lost:
To repro:
1. Run some query e.g:
```sql
SELECT
channel,
COUNT(*) AS "Count"
FROM wikipedia
GROUP BY 1
ORDER BY 2 DESC
```
2. Edit the query somehow, say change "Count" to "CountBlah"
3. Use the table actions to modify the query:
Notice the query edit of "Blah" is lost as the table action overwrites the query completely.
Instead it should now provide an action (a function that takes a query and manipulates it) instead so the action is simply applied to the edited query.
This PR also:
- Skips the java check in the helper script to make JDK11 dev easier
- Simplifies the format guesser
- Adds and improves react hooks
This PR has:
- [x] been self-reviewed.
- [x] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #12037: Web console: use query actions in query view
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #12037:
URL: https://github.com/apache/druid/pull/12037#discussion_r764774572
##########
File path: web-console/src/utils/query-manager.tsx
##########
@@ -86,40 +110,85 @@ export class QueryManager<Q, R> {
const cancelToken = new axios.CancelToken(cancelFn => {
this.currentRunCancelFn = cancelFn;
});
- this.processQuery(this.lastQuery, cancelToken, (intermediateQuery: any) => {
- this.lastIntermediateQuery = intermediateQuery;
- }).then(
- data => {
- if (this.currentQueryId !== myQueryId) return;
- this.currentRunCancelFn = undefined;
+
+ const query = this.lastQuery;
+ let data: R | IntermediateQueryState<I>;
+ try {
+ data = await this.processQuery(query, cancelToken, (intermediateQuery: any) => {
+ this.lastIntermediateQuery = intermediateQuery;
+ });
+ } catch (e) {
+ if (this.currentQueryId !== myQueryId) return;
+ this.currentRunCancelFn = undefined;
+ this.setState(
+ new QueryState<R, E>({
+ error: axios.isCancel(e) ? new Error(`canceled.`) : e, // ToDo: why does the cancel error get remapped?
Review comment:
did you intend to resolve this todo in some way?
##########
File path: web-console/src/utils/query-manager.tsx
##########
@@ -86,40 +110,85 @@ export class QueryManager<Q, R> {
const cancelToken = new axios.CancelToken(cancelFn => {
this.currentRunCancelFn = cancelFn;
});
- this.processQuery(this.lastQuery, cancelToken, (intermediateQuery: any) => {
- this.lastIntermediateQuery = intermediateQuery;
- }).then(
- data => {
- if (this.currentQueryId !== myQueryId) return;
- this.currentRunCancelFn = undefined;
+
+ const query = this.lastQuery;
+ let data: R | IntermediateQueryState<I>;
+ try {
+ data = await this.processQuery(query, cancelToken, (intermediateQuery: any) => {
+ this.lastIntermediateQuery = intermediateQuery;
+ });
+ } catch (e) {
+ if (this.currentQueryId !== myQueryId) return;
+ this.currentRunCancelFn = undefined;
+ this.setState(
+ new QueryState<R, E>({
+ error: axios.isCancel(e) ? new Error(`canceled.`) : e, // ToDo: why does the cancel error get remapped?
+ lastData: this.state.getSomeData(),
+ }),
+ );
+ return;
+ }
+
+ let bacgroundChecks = 0;
+ while (data instanceof IntermediateQueryState) {
+ try {
+ if (!this.backgroundStatusCheck) {
+ throw new Error(
+ 'backgroundStatusCheck must be set in intermediate query state is returned',
+ );
+ }
+ cancelToken.throwIfRequested();
+
this.setState(
- new QueryState<R>({
- data,
+ new QueryState<R, E, I>({
+ loading: true,
+ intermediate: data.state,
lastData: this.state.getSomeData(),
}),
);
- },
- (e: any) => {
+
+ const delay =
+ data.delay ??
+ (bacgroundChecks > 0
+ ? this.backgroundStatusCheckDelay
+ : this.backgroundStatusCheckInitDelay);
+
+ if (delay) {
+ await wait(delay);
+ cancelToken.throwIfRequested();
+ }
+
+ data = await this.backgroundStatusCheck(data.state, query, cancelToken);
+ } catch (e) {
if (this.currentQueryId !== myQueryId) return;
this.currentRunCancelFn = undefined;
- if (axios.isCancel(e)) {
- e = new Error(`canceled.`); // ToDo: fix!
- }
this.setState(
- new QueryState<R>({
- error: e,
+ new QueryState<R, E>({
+ error: axios.isCancel(e) ? new Error(`canceled.`) : e, // ToDo: why does the cancel error get remapped?
Review comment:
same question re: todo comment
##########
File path: web-console/script/druid
##########
@@ -104,6 +104,7 @@ function start() {
_error "${DRUID_PID_FILE} exists with pid '$(<${DRUID_PID_FILE})'. Either shutdown druid or delete this file."
fi
+ export DRUID_SKIP_JAVA_CHECK=1
Review comment:
what is this change for?
##########
File path: web-console/src/utils/query-manager.tsx
##########
@@ -86,40 +110,85 @@ export class QueryManager<Q, R> {
const cancelToken = new axios.CancelToken(cancelFn => {
this.currentRunCancelFn = cancelFn;
});
- this.processQuery(this.lastQuery, cancelToken, (intermediateQuery: any) => {
- this.lastIntermediateQuery = intermediateQuery;
- }).then(
- data => {
- if (this.currentQueryId !== myQueryId) return;
- this.currentRunCancelFn = undefined;
+
+ const query = this.lastQuery;
+ let data: R | IntermediateQueryState<I>;
+ try {
+ data = await this.processQuery(query, cancelToken, (intermediateQuery: any) => {
+ this.lastIntermediateQuery = intermediateQuery;
+ });
+ } catch (e) {
+ if (this.currentQueryId !== myQueryId) return;
+ this.currentRunCancelFn = undefined;
+ this.setState(
+ new QueryState<R, E>({
+ error: axios.isCancel(e) ? new Error(`canceled.`) : e, // ToDo: why does the cancel error get remapped?
+ lastData: this.state.getSomeData(),
+ }),
+ );
+ return;
+ }
+
+ let bacgroundChecks = 0;
Review comment:
nit: bacgroundChecks -> backgroundChecks
##########
File path: web-console/src/utils/query-manager.tsx
##########
@@ -86,40 +110,85 @@ export class QueryManager<Q, R> {
const cancelToken = new axios.CancelToken(cancelFn => {
this.currentRunCancelFn = cancelFn;
});
- this.processQuery(this.lastQuery, cancelToken, (intermediateQuery: any) => {
- this.lastIntermediateQuery = intermediateQuery;
- }).then(
- data => {
- if (this.currentQueryId !== myQueryId) return;
- this.currentRunCancelFn = undefined;
+
+ const query = this.lastQuery;
+ let data: R | IntermediateQueryState<I>;
+ try {
+ data = await this.processQuery(query, cancelToken, (intermediateQuery: any) => {
+ this.lastIntermediateQuery = intermediateQuery;
+ });
+ } catch (e) {
+ if (this.currentQueryId !== myQueryId) return;
+ this.currentRunCancelFn = undefined;
+ this.setState(
+ new QueryState<R, E>({
+ error: axios.isCancel(e) ? new Error(`canceled.`) : e, // ToDo: why does the cancel error get remapped?
+ lastData: this.state.getSomeData(),
+ }),
+ );
+ return;
+ }
+
+ let bacgroundChecks = 0;
+ while (data instanceof IntermediateQueryState) {
+ try {
+ if (!this.backgroundStatusCheck) {
+ throw new Error(
+ 'backgroundStatusCheck must be set in intermediate query state is returned',
Review comment:
should this be 'must be set _if_ intermediate query state ...'
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky merged pull request #12037: Web console: use query actions in query view
Posted by GitBox <gi...@apache.org>.
vogievetsky merged pull request #12037:
URL: https://github.com/apache/druid/pull/12037
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org