Add pageable data widget

Add pageable data widget

This allows the user to request more data if they need it.
This has been integrated into bot-page and can be added
to bot-list and task-list as well.

So far this seems to speed up the bot-page a bit because
the initial query for data is smaller.  It seems that the
previous logic of paging for the last 200 tasks was the most
expensive part of the whole page.

BUG=631047
Committed: https://github.com/luci/luci-py/commit/ccab439ed6c9fd19044e6b00945219c991d935e8

[kjlubick, 2016-09-27 19:42:29]

Description was changed from

==========
Add pageable data widget

BUG=631047
==========

to

==========
Add pageable data widget

This allows the user to request more data if they need it.
This has been integrated into bot-page and can be added
to bot-list and task-list as well.

So far this seems to speed up the bot-page a bit because
the initial query for data is smaller.  It seems that the
previous logic of paging for the last 200 tasks was the most
expensive part of the whole page.

BUG=631047
==========

[kjlubick, 2016-09-27 19:44:05]

kjlubick@google.com changed reviewers:
+ stephana@google.com

[kjlubick, 2016-09-27 19:44:06]

https://screenshot.googleplex.com/wz4JZFtGn58

Play with at
https://2365-bd022ad-tainted-kjlubick-dot-chromium-swarm-dev.appspot.com/newu...

[stephana, 2016-09-28 13:25:23]

Overall this looks good, lots of small nits.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:287:
hidden$="[[_truthy(_selected)]]"
who does 'selected' alone not work here ?

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:355: _parseEvents: {
This don't need to be defined in properties. Why can't you just define them as
functions on the element below.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/pageable-data.html (right):

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:36: load(url,
headers, pageSize): Initialize the element and make a call to
empty line between the method descriptions and properties to make it easier to
read.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:107: this._cursor
= undefined;
I would either delete the _cursor member or set it to null (preferable).
Setting it to 'undefined' IMO is a contradiction of the meaning of undefined.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:112:
console.log("url can't be blank");
Same as below. Assume that the client of this element uses it correctly
according to the contract you outlined in the description.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:130:
console.log("You must first call load() before calling page()");
console.log IMO is mostly for debugging purposes and should be used sparingly in
production. Same below. 

You could throw an error, since this is a programmer's error, who is not
following the contract laid out in the description. 

You can also assume that page() is not called before the element is properly
initialized, since you stated clearly that load() needs to be called first.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:136: if
(this._cursor !== undefined) {
I would make _cursor well defined in the sense, that it's either null or a valid
value that is truthy. Initialize it to null in the very beginning and set it to
null when it's cleared. But make it so that _cursor is always has a well defined
value. 

Then this could be just 

   if (this._cursor) { ....

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:151:
this.set("busy", false);
There should be a notification of failure shown to the user other than the log
(which is usually hidden).

[kjlubick, 2016-09-28 13:50:37]

Also added docs for the now public methods of bot-page-data

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:287:
hidden$="[[_truthy(_selected)]]"
On 2016/09/28 at 13:25:23, stephana wrote:
> who does 'selected' alone not work here ?

I'm not entirely sure, but it seems that when _selected is the string "0", i.e.
show tasks, hidden$= evaluates to true.  This hack seems to make it properly be
false.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:355: _parseEvents: {
On 2016/09/28 at 13:25:23, stephana wrote:
> This don't need to be defined in properties. Why can't you just define them as
functions on the element below.

I tried defining the functions below, but Polymer didn't pass them as I expected
into pageable-data.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/common/pageable-data.html (right):

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:36: load(url,
headers, pageSize): Initialize the element and make a call to
On 2016/09/28 at 13:25:23, stephana wrote:
> empty line between the method descriptions and properties to make it easier to
read.

Done.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:107: this._cursor
= undefined;
On 2016/09/28 at 13:25:23, stephana wrote:
> I would either delete the _cursor member or set it to null (preferable).
> Setting it to 'undefined' IMO is a contradiction of the meaning of undefined.

Done.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:112:
console.log("url can't be blank");
On 2016/09/28 at 13:25:23, stephana wrote:
> Same as below. Assume that the client of this element uses it correctly
according to the contract you outlined in the description.

Done.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:130:
console.log("You must first call load() before calling page()");
On 2016/09/28 at 13:25:23, stephana wrote:
> console.log IMO is mostly for debugging purposes and should be used sparingly
in production. Same below. 
> 
> You could throw an error, since this is a programmer's error, who is not
following the contract laid out in the description. 
> 
> You can also assume that page() is not called before the element is properly
initialized, since you stated clearly that load() needs to be called first.

Done.  I made it a throw just to be safe.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:136: if
(this._cursor !== undefined) {
On 2016/09/28 at 13:25:23, stephana wrote:
> I would make _cursor well defined in the sense, that it's either null or a
valid value that is truthy. Initialize it to null in the very beginning and set
it to null when it's cleared. But make it so that _cursor is always has a well
defined value. 
> 
> Then this could be just 
> 
>    if (this._cursor) { ....

Done.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/common/pageable-data.html:151:
this.set("busy", false);
On 2016/09/28 at 13:25:23, stephana wrote:
> There should be a notification of failure shown to the user other than the log
(which is usually hidden).

Done.

[stephana, 2016-09-28 15:45:33]

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:287:
hidden$="[[_truthy(_selected)]]"
On 2016/09/28 13:50:36, kjlubick wrote:
> On 2016/09/28 at 13:25:23, stephana wrote:
> > who does 'selected' alone not work here ?
> 
> I'm not entirely sure, but it seems that when _selected is the string "0",
i.e.
> show tasks, hidden$= evaluates to true.  This hack seems to make it properly
be
> false.

if _selected is string "0", _truthy will return true. And in this scenario
_selected should always an integer, so it can be used directly.

https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
appengine/swarming/elements/res/imp/botpage/bot-page.html:355: _parseEvents: {
On 2016/09/28 13:50:36, kjlubick wrote:
> On 2016/09/28 at 13:25:23, stephana wrote:
> > This don't need to be defined in properties. Why can't you just define them
as
> functions on the element below.
> 
> I tried defining the functions below, but Polymer didn't pass them as I
expected
> into pageable-data.

Acknowledged.

[kjlubick, 2016-09-28 15:49:28]

On 2016/09/28 at 15:45:33, stephana wrote:
>
https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
> File appengine/swarming/elements/res/imp/botpage/bot-page.html (right):
> 
>
https://codereview.chromium.org/2372323002/diff/40001/appengine/swarming/elem...
> appengine/swarming/elements/res/imp/botpage/bot-page.html:287:
hidden$="[[_truthy(_selected)]]"
> On 2016/09/28 13:50:36, kjlubick wrote:
> > On 2016/09/28 at 13:25:23, stephana wrote:
> > > who does 'selected' alone not work here ?
> > 
> > I'm not entirely sure, but it seems that when _selected is the string "0",
i.e.
> > show tasks, hidden$= evaluates to true.  This hack seems to make it properly
be
> > false.
> 
> if _selected is string "0", _truthy will return true. And in this scenario
_selected should always an integer, so it can be used directly.
> 

I know it it should be the integer 0 - but it is the string "0".  If I don't
have the call to truthy, the element is always hidden.

[kjlubick, 2016-09-28 17:25:25]

I added a _showEvents as we discussed.

[kjlubick, 2016-09-28 19:08:17]

On 2016/09/28 at 17:25:25, kjlubick wrote:
> I added a _showEvents as we discussed.

Doc added about hidden$= bug/feature

[stephana, 2016-09-28 19:24:14]

On 2016/09/28 19:08:17, kjlubick wrote:
> On 2016/09/28 at 17:25:25, kjlubick wrote:
> > I added a _showEvents as we discussed.
> 
> Doc added about hidden$= bug/feature

lgtm

[KevinL, 2016-09-28 19:24:54]

kjlubick@chromium.org changed reviewers:
+ kjlubick@chromium.org

[KevinL, 2016-09-28 19:24:55]

lgtm

[KevinL, 2016-09-28 19:24:58]

The CQ bit was checked by kjlubick@chromium.org

[commit-bot: I haz the power, 2016-09-28 19:25:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 19:28:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-28 19:28:36]

Try jobs failed on following builders:
  Luci-py Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/318b15194333d310)

[kjlubick, 2016-09-28 19:35:50]

The CQ bit was checked by kjlubick@google.com

[kjlubick, 2016-09-28 19:35:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjlubick@chromium.org, stephana@google.com
Link to the patchset: https://codereview.chromium.org/2372323002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-28 19:36:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 19:39:17]

Description was changed from

==========
Add pageable data widget

This allows the user to request more data if they need it.
This has been integrated into bot-page and can be added
to bot-list and task-list as well.

So far this seems to speed up the bot-page a bit because
the initial query for data is smaller.  It seems that the
previous logic of paging for the last 200 tasks was the most
expensive part of the whole page.

BUG=631047
==========

to

==========
Add pageable data widget

This allows the user to request more data if they need it.
This has been integrated into bot-page and can be added
to bot-list and task-list as well.

So far this seems to speed up the bot-page a bit because
the initial query for data is smaller.  It seems that the
previous logic of paging for the last 200 tasks was the most
expensive part of the whole page.

BUG=631047

Committed:
https://github.com/luci/luci-py/commit/ccab439ed6c9fd19044e6b00945219c991d935e8
==========

[commit-bot: I haz the power, 2016-09-28 19:39:19]

Committed patchset #7 (id:120001) as
https://github.com/luci/luci-py/commit/ccab439ed6c9fd19044e6b00945219c991d935e8