IGNITE-11035 Updates in queries page. #6021

vsisko · 2019-02-05T03:48:18Z

No description provided.

…nto ignite-11035

Klaster1 · 2019-02-05T05:36:16Z

modules/web-console/frontend/app/modules/agent/AgentManager.service.js

     * @param {Boolean} [lazy] query flag.
     * @param {Boolean} [collocated] Collocated query.
-     * @returns {Promise}
+     * @returns {Promise<VisorQueryResult>}


@vsisko I couldn't find where VisorQueryResult is defined.

VisorQueryResult is a Java class and doesn't has analog in JS.

That's not how types in JSDoc or TS work. You are supposed to describe the data structure so tooling and fellow developers are aware what's inside. This concerns all data received from backend.

The returned type description has been added.

Klaster1 · 2019-02-05T05:38:16Z

modules/web-console/frontend/app/components/cluster-selector/controller.js

-        this.agentMgr.switchCluster(this.cluster);
+    change(item) {
+        this.agentMgr.switchCluster(this.cluster)
+            .then(() => this.cluster = item)


Updated AgentManager.switchCluster returns true if cluster was successful switched, did you check that assigning true to this.cluster doesn't break cluster-selector?

We set to a cluster value of change function argument. Not result of switchCluster.
Removed true result on resolve of switchCluster function

Klaster1 · 2019-02-05T05:42:06Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

    }
+
+    toJSON() {
+        return {


Klaster1 · 2019-02-05T06:07:33Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts


-        $window.addEventListener('beforeunload', () => {
-            this._closeOpenedQueries(this.$scope.notebook.paragraphs);
+        const offTransitions = $transitions.onBefore({from: 'base.sql.notebook'}, ($transition$) => {


Did you mean this.offTransitions = ?

Changed to this.offTransitions

Klaster1 · 2019-02-05T06:08:07Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

+        const offTransitions = $transitions.onBefore({from: 'base.sql.notebook'}, ($transition$) => {
+            const options = $transition$.options();
+
+            if (options.custom.justIDUpdate || options.redirectedFrom)


What's the point of this check? Looks like a copy/paste to me.

Removed unused check.

Klaster1 · 2019-02-05T06:12:07Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

-            await this.Confirm.confirm('Are you sure you want to remove query: "' + paragraph.name + '"?');
-            this.$scope.stopRefresh(paragraph);
+            const msg = (this._hasRunningQueries([paragraph])
+                ? 'Query is being executed. Are you sure you want cancel and remove query: "'


"you want to cancel"

Typo is fixed.

Klaster1 · 2019-02-05T06:16:52Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

        }
    }

    async removeParagraph(paragraph: Paragraph) {


When removing instances of Paragraph, you might want to dispose of Subjects by calling Subject.complete.

Added closing of the subjects.

…nto ignite-11035

Klaster1 · 2019-02-06T09:40:29Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

                    this.clusterIsAvailable = (!!cluster && cluster.active === true) || agentMgr.isDemoMode();
                })
            );



Please add comment as to why redirectedFrom matters.

Added comment.

Klaster1 · 2019-02-06T09:42:42Z

modules/web-console/frontend/app/modules/agent/AgentManager.service.js


                this.connectionSbj.next(state);

                this.saveToStorage(cluster);


You can reuse type definitions using @typedef or import from TS file. Ask me how to.

Surely there's no List in parsed JSON. Please use correct types.

Fixed jsdock.

…nto ignite-11035

nva · 2019-03-11T03:28:32Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

+                    onQueryStarted(res);
+                    $scope.$applyAsync();
+                }),
+                expand((res) => {


I think that an expand, filter is not really needed and can be replaced with something like this:

exhaustMap((res) => { if (!_.isNil(res.rows)) return from(res); return merge(fetchFirstPageTask, pingQueryTask); })

@Klaster1 what do you think about this?

Yeah, I'm not sure recursion is even require here. The exhaust block basically polls first page results and pings to keep the polling alive. If I understand the query API correctly, res.queryId - the only value received from previous recursion call - does not change ever.

Recursion is removed by changing of exexpand to exhaustMap.

nva · 2019-03-11T03:33:41Z

modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryPingTask.java

+            if (debug)
+                log(ignite.log(), "Ping of query finished: " + qryId, getClass(), start);
+
+            return new VisorEither<>(new VisorQueryPingTaskResult(true));


The inProgress flag is always true, I think it should be removed

Removed unused task result property.

Klaster1 · 2019-03-11T05:33:09Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

+                    onQueryStarted(res);
+                    $scope.$applyAsync();
+                }),
+                expand((res) => {


Yeah, I'm not sure recursion is even require here. The exhaust block basically polls first page results and pings to keep the polling alive. If I understand the query API correctly, res.queryId - the only value received from previous recursion call - does not change ever.

Klaster1 · 2019-03-11T05:36:57Z

...s/web-console/frontend/app/components/page-queries/components/queries-notebook/controller.ts

-
-                        _showLoading(paragraph, false);
-                        delete paragraph.subscription;
+                first()


@vsisko given that first and take(1) are slightly different, do you really intend to throw if no results arrive upon observable completion? If user cancels the query by leaving the screen, wouldn't it throw?

I'm just curious here.

On cancel of the query no error is thrown.

(cherry picked from commit c2da3d5)

…Fixes apache#6021. (cherry picked from commit c2da3d5)

vsisko added 5 commits January 24, 2019 18:44

IGNITE-11035 Use RXJS for query execution.

22d1641

IGNITE-11035 Use RXJS for query execution.

7a59244

IGNITE-11035 Use RXJS for query execution.

cbeac05

Merge branch 'master' of https://gitbox.apache.org/repos/asf/ignite i…

258f536

…nto ignite-11035

IGNITE-11035 Confirmations.

525c57b

Klaster1 suggested changes Feb 5, 2019

View reviewed changes

vsisko added 3 commits February 6, 2019 10:41

Merge branch 'master' of https://gitbox.apache.org/repos/asf/ignite i…

f3de7b4

…nto ignite-11035

IGNITE-11035 Merged with master.

e5dd2a4

IGNITE-11035 Small fixes.

f3d972d

Klaster1 suggested changes Feb 6, 2019

View reviewed changes

IGNITE-11035 Merged with master.

5751a8b

Klaster1 approved these changes Feb 6, 2019

View reviewed changes

vsisko added 3 commits February 8, 2019 13:51

Merge branch 'master' of https://gitbox.apache.org/repos/asf/ignite i…

a66367a

…nto ignite-11035

IGNITE-11035 Merged with master.

1fceb6d

IGNITE-11035 Fixed unlock of paragraph on query cancel.

0455b8e

Klaster1 approved these changes Feb 18, 2019

View reviewed changes

vsisko added 4 commits February 26, 2019 15:56

IGNITE-11035 Merged with master.

61f480c

IGNITE-11035 Merged with master.

62c50f7

Merge branch 'master' of https://gitbox.apache.org/repos/asf/ignite i…

905e8fe

…nto ignite-11035

IGNITE-11496 Periodical notify of node when query is long.

90023fa

nva requested changes Mar 11, 2019

View reviewed changes

IGNITE-11496 Removed unused argument.

d1c46d1

Klaster1 suggested changes Mar 11, 2019

View reviewed changes

IGNITE-11496 expand to exhaustMap.

d51e98e

nva approved these changes Mar 11, 2019

View reviewed changes

asfgit closed this in c2da3d5 Mar 11, 2019

nva pushed a commit to gridgain/apache-ignite that referenced this pull request Mar 12, 2019

IGNITE-11035 Updates in queries page - Fixes apache#6021.

82f277c

(cherry picked from commit c2da3d5)

nva pushed a commit to gridgain/apache-ignite that referenced this pull request Mar 12, 2019

IGNITE-11035 Updates in queries page - Fixes apache#6021.

0db9f72

(cherry picked from commit c2da3d5)

akuznetsov-gridgain deleted the ignite-11035 branch March 19, 2019 15:02

akuznetsov-gridgain pushed a commit to gridgain/apache-ignite that referenced this pull request Mar 19, 2019

IGN-13735 Backport IGNITE-11496/IGNITE-11035 Updates in queries page - …

8adbfa5

…Fixes apache#6021. (cherry picked from commit c2da3d5)

IGNITE-11035 Updates in queries page. #6021

IGNITE-11035 Updates in queries page. #6021

Uh oh!

Conversation

vsisko commented Feb 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants