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/09/21 23:14:28 UTC
[GitHub] drill pull request #953: DRILL-5259: Allow listing a user-defined number of ...
GitHub user kkhatua opened a pull request:
https://github.com/apache/drill/pull/953
DRILL-5259: Allow listing a user-defined number of profiles
Added an additional field in the UI allowing a user to specify the max number of profiles to display
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kkhatua/drill DRILL-5259
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/953.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 #953
----
commit 276f8cb72412d5bcb0b88c0616e65a04dc7f1b52
Author: Kunal Khatua <kk...@maprtech.com>
Date: 2017-09-21T23:03:41Z
DRILL-5259: Allow listing a user-defined number of profiles
Added an additional field in the UI allowing a user to specify the max number of profiles to display
----
---
[GitHub] drill pull request #953: DRILL-5259: Allow listing a user-defined number of ...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/953#discussion_r140644680
--- Diff: exec/java-exec/src/main/resources/rest/profile/list.ftl ---
@@ -37,7 +37,15 @@
<strong>No running queries.</strong>
</div>
</#if>
- <h3>Completed Queries</h3>
+ <table width="100%">
+ <tr>
+ <td><h3>Completed Queries</h3></td>
+ <td align="right">
+ <form action="/profiles" method="get"><span title="Max number of profiles to list"># Profiles: </span>
+ <input type="text" size="7" name="max" value="20">
+ <input type="submit" value="Fetch">
+ </form></td>
+ </tr></table>
--- End diff --
Good idea. Some thoughts:
1. Do we need a table to display one line?
2. The value of the input is always 20. But, isn't our default something like 100? And, if I choose to display 50, shouldn't that be the default when the page is redisplayed?
3. This change will render a UI something like this:
```
Completed Queries
Completed Queries # Profiles [20 ] [Fetch]
<table of profiles>
```
This seems a bit busy. Can we do it like this:
```
Completed Queries Show [ 20] [Go]
```
That is, with the form right-aligned on the same line as the title.
Better, can we implement paging? Rather than display 200 or so profiles (which is too much for humans to digest), show 10 or 20, and provide the usual buttons to display multiple pages.
Doing it this way requires a filter on top of the fetch logic, but should not be too hard. Might be inefficient to fetch, say, 200 profiles, and keep only the last 20, but that is our internal problem, not the user's problem. We can optimize that logic later.
---
[GitHub] drill issue #953: DRILL-5259: Allow listing a user-defined number of profile...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/953
Closed the original PR accidentally. Have filed a new one #963
---
[GitHub] drill issue #953: DRILL-5259: Allow listing a user-defined number of profile...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on the issue:
https://github.com/apache/drill/pull/953
@paul-rogers since you had reviewed the initial PR, thought you should review this minor update as well.
---
[GitHub] drill pull request #953: DRILL-5259: Allow listing a user-defined number of ...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua closed the pull request at:
https://github.com/apache/drill/pull/953
---
[GitHub] drill pull request #953: DRILL-5259: Allow listing a user-defined number of ...
Posted by kkhatua <gi...@git.apache.org>.
Github user kkhatua commented on a diff in the pull request:
https://github.com/apache/drill/pull/953#discussion_r140927942
--- Diff: exec/java-exec/src/main/resources/rest/profile/list.ftl ---
@@ -37,7 +37,15 @@
<strong>No running queries.</strong>
</div>
</#if>
- <h3>Completed Queries</h3>
+ <table width="100%">
+ <tr>
+ <td><h3>Completed Queries</h3></td>
+ <td align="right">
+ <form action="/profiles" method="get"><span title="Max number of profiles to list"># Profiles: </span>
+ <input type="text" size="7" name="max" value="20">
+ <input type="submit" value="Fetch">
+ </form></td>
+ </tr></table>
--- End diff --
@paul-rogers I agree. Ideally, this should all be paginated, but we're a while away from that.
I'll modify the commit to display
```
Completed Queries Show [ 20] [Go]
```
And remove mentioning the value of 20. This modification came up because Saurabh couldn't figure out the option of reloading more profiles until he asked me.
---