You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by kkhatua <gi...@git.apache.org> on 2017/11/09 02:31:12 UTC
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
GitHub user kkhatua opened a pull request:
https://github.com/apache/drill/pull/1029
DRILL-5867: List profiles in pages rather than a long verbose listing
Leverage existing DataTables libraries to paginate a long pre-fetched list of profiles for listing.
Added benefit of querying through that list (search field) is also available for a user
Minor change made to the display text for prefetching of profiles (DRILL-5259) so that it is not confused with what this commit adds to the UI.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kkhatua/drill DRILL-5867
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1029.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1029
----
commit 68d09a506d37134b660437260ef0fd4ca5ce3f9d
Author: Kunal Khatua <kk...@maprtech.com>
Date: 2017-11-09T02:17:19Z
DRILL-5867: List profiles in pages rather than a long verbose listing
Leverage existing DataTables libraries to paginate a long pre-fetched list of profiles for listing.
Added benefit of querying through that list (search field) is also available for a user
Minor change made to the display text for prefetching of profiles (DRILL-5259) so that it is not confused with what this commit adds to the UI.
----
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150938880
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
+ //Extract upto max char limit as snippet
+ String sizeCappedQuerySnippet = queryText.substring(0, Math.min(queryText.length(), QUERY_SNIPPET_MAX_CHAR));
+ //Trimming down based on line-count
+ if ( QUERY_SNIPPET_MAX_LINES < sizeCappedQuerySnippet.split(System.lineSeparator()).length ) {
--- End diff --
+1
---
[GitHub] drill issue #1029: DRILL-5867: List profiles in pages rather than a long ver...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1029
+1, LGTM.
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1029
---
[GitHub] drill issue #1029: DRILL-5867: List profiles in pages rather than a long ver...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1029
@arina-ielchiieva / @paul-rogers Please review this.
---
[GitHub] drill issue #1029: DRILL-5867: List profiles in pages rather than a long ver...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1029
Snapshot when rendering the defaults (10 per page) from a pre-loaded set of the latest 123 profiles
The query snippet is restricted to 8 lines at most and indicates if there is more to the query text with a trailing set of `...`
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150085841
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
--- End diff --
1. I usually place private method int he end of of the class.
2. We can add javadoc here explaining that first we limit original query size and if size fits but query has too many lines we limit it as well for better readability on Web UI.
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150938808
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
+ //Extract upto max char limit as snippet
+ String sizeCappedQuerySnippet = queryText.substring(0, Math.min(queryText.length(), QUERY_SNIPPET_MAX_CHAR));
+ //Trimming down based on line-count
+ if ( QUERY_SNIPPET_MAX_LINES < sizeCappedQuerySnippet.split(System.lineSeparator()).length ) {
+ int linesConstructed = 0;
+ StringBuilder lineCappedQuerySnippet = new StringBuilder();
+ String[] queryParts = sizeCappedQuerySnippet.split(System.lineSeparator());
+ for (String qPart : queryParts) {
+ lineCappedQuerySnippet.append(qPart);
+ if ( ++linesConstructed < QUERY_SNIPPET_MAX_LINES ) {
+ lineCappedQuerySnippet.append(System.lineSeparator());
--- End diff --
I wanted to preserve the original query format, hence, I'm applying new line instead of spaces. Besides, the snippet can also carry single-line comments, and putting them into 1 line will make it appear that anything following a `--` is all comments
---
[GitHub] drill issue #1029: DRILL-5867: List profiles in pages rather than a long ver...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/1029
Snapshot when testing with search filter for FAILED queries and navigating to page 2 of that list. Information about the number of filtered items, etc is also provided.
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150086785
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
+ //Extract upto max char limit as snippet
+ String sizeCappedQuerySnippet = queryText.substring(0, Math.min(queryText.length(), QUERY_SNIPPET_MAX_CHAR));
+ //Trimming down based on line-count
+ if ( QUERY_SNIPPET_MAX_LINES < sizeCappedQuerySnippet.split(System.lineSeparator()).length ) {
+ int linesConstructed = 0;
+ StringBuilder lineCappedQuerySnippet = new StringBuilder();
+ String[] queryParts = sizeCappedQuerySnippet.split(System.lineSeparator());
+ for (String qPart : queryParts) {
+ lineCappedQuerySnippet.append(qPart);
+ if ( ++linesConstructed < QUERY_SNIPPET_MAX_LINES ) {
+ lineCappedQuerySnippet.append(System.lineSeparator());
--- End diff --
Do we want to append with new line or maybe space for better readability?
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150086018
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
+ //Extract upto max char limit as snippet
+ String sizeCappedQuerySnippet = queryText.substring(0, Math.min(queryText.length(), QUERY_SNIPPET_MAX_CHAR));
+ //Trimming down based on line-count
+ if ( QUERY_SNIPPET_MAX_LINES < sizeCappedQuerySnippet.split(System.lineSeparator()).length ) {
--- End diff --
1. We can create variable for `sizeCappedQuerySnippet.split(System.lineSeparator())` so we do split only once.
2. Please remove spaces in `if` clause: `if ( QUERY_SNIPPET_MAX_LINES < sizeCappedQuerySnippet.split(System.lineSeparator()).length ) {` -> `if (QUERY_SNIPPET_MAX_LINES < splittedQuery.length) {` and in `if ( ++linesConstructed < QUERY_SNIPPET_MAX_LINES ) {` in the code below.
---
[GitHub] drill pull request #1029: DRILL-5867: List profiles in pages rather than a l...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:
https://github.com/apache/drill/pull/1029#discussion_r150938917
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ---
@@ -93,13 +96,35 @@ public ProfileInfo(DrillConfig drillConfig, String queryId, long startTime, long
this.time = new Date(startTime);
this.foreman = foreman;
this.link = generateLink(drillConfig, foreman, queryId);
- this.query = query.substring(0, Math.min(query.length(), 150));
+ this.query = extractQuerySnippet(query);
this.state = state;
this.user = user;
this.totalCost = totalCost;
this.queueName = queueName;
}
+ private String extractQuerySnippet(String queryText) {
--- End diff --
+1
Will do this
---