You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2020/04/22 12:29:00 UTC
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15784
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
[www] convert human-readable int to int when compare numeric strings
The /tables page show `tablet_count` as a HumanReadableInt, when sort
tables by tablet_count, human-readable int should be converted to int.
Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
---
M www/kudu.js
1 file changed, 30 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15784/1
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@56
PS1, Line 56: val = parseFloat(humanReadableNum);
> Don't we need to substring this like we do above?
If value < 1000 or value > 1E15, the numeric string doesn't end up with a special charactor that we need to exclude.
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@60
PS1, Line 60: if (end == 'k') {
> Nit: consider a switch like in toNumBytes()?
If use switch, there would be too many cases, like:
case '0':
case '1':
...
case '9':
break;
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@68
PS1, Line 68: } else if (isNaN(end)) {
> Nit: could you comment why we are stopping here, rather than going above wi
Here I borrowed from HumanReadableInt::ToInt64, it reverses HumanReadableInt::ToString, which we used for webUI.
In HumanReadableInt::ToString, number bigger than 1E15 use scientific notation.
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 Apr 2020 07:30:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
Patch Set 1:
So for something like 10GB v/s 100MB, 10GB looks larger with number of digits, right?
Personally I don't see a need for this unless the output will be fed to a `sort` utility. But this is GUI so there is no such expectation.
Perhaps you could link screenshots to make the case for this change.
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 23 Apr 2020 02:03:01 +0000
Gerrit-HasComments: No
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
Patch Set 2:
(1 comment)
> Patch Set 2: Code-Review+2
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@56
PS1, Line 56: val = parseFloat(humanReadableNum);
> I see, and it looks like parseFloat() actually ignores any trailing non-num
Yes. https://www.w3schools.com/jsref/jsref_parsefloat.asp
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 24 Apr 2020 03:50:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
[www] convert human-readable int to int when compare numeric strings
The /tables page show `tablet_count` as a HumanReadableInt, when sort
tables by tablet_count, human-readable int should be converted to int.
Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Reviewed-on: http://gerrit.cloudera.org:8080/15784
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M www/kudu.js
1 file changed, 33 insertions(+), 3 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
Patch Set 1:
(3 comments)
> Patch Set 1:
>
> So for something like 10GB v/s 100MB, 10GB looks larger with number of digits, right?
> Personally I don't see a need for this unless the output will be fed to a `sort` utility. But this is GUI so there is no such expectation.
>
> Perhaps you could link screenshots to make the case for this change.
Some tables in the UI are sortable as of 1.12.
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@56
PS1, Line 56: val = parseFloat(humanReadableNum);
Don't we need to substring this like we do above?
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@60
PS1, Line 60: if (end == 'k') {
Nit: consider a switch like in toNumBytes()?
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@68
PS1, Line 68: } else if (isNaN(end)) {
Nit: could you comment why we are stopping here, rather than going above with Q and beyond?
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 23 Apr 2020 02:15:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15784
to look at the new patch set (#2).
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
[www] convert human-readable int to int when compare numeric strings
The /tables page show `tablet_count` as a HumanReadableInt, when sort
tables by tablet_count, human-readable int should be converted to int.
Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
---
M www/kudu.js
1 file changed, 33 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15784/2
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [www] convert human-readable int to int when compare numeric strings
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15784 )
Change subject: [www] convert human-readable int to int when compare numeric strings
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/15784/1/www/kudu.js@56
PS1, Line 56: val = parseFloat(humanReadableNum);
> If value < 1000 or value > 1E15, the numeric string doesn't end up with a s
I see, and it looks like parseFloat() actually ignores any trailing non-numeric strings.
--
To view, visit http://gerrit.cloudera.org:8080/15784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7e832beb0dfdf5c2162bf0e9faeb3d0d0d737cd
Gerrit-Change-Number: 15784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 23 Apr 2020 08:12:10 +0000
Gerrit-HasComments: Yes