V2: review request for "Disk IO". "CPU Usage", and "Build Time" pages

* V2: review request for "Disk IO". "CPU Usage", and "Build Time" pages
@ 2014-02-12  1:11 Reyna, David
  2014-02-14 13:39 ` Barros Pena, Belen
  0 siblings, 1 reply; 8+ messages in thread
From: Reyna, David @ 2014-02-12  1:11 UTC (permalink / raw)
  To: Barros Pena, Belen; +Cc: toaster

Hi Belen,

New push for review: "dreyna/time-disk-cpu-view"

* Column Ordering

Ignore my previous comment. After investigation I realize what I needed to do. I was missing a flag for " _get_toggle_order_icon" for the three fields that are supposed to be naturally in descending order, and I did not realize it at first because it was not in the original All Task page code.

This is now fixed.

* Sorting in Edit Columns list

Done.

* Filter dialog: title and "All Tasks" entry

Done.

So, I believe that I have addressed all outstanding issues, and that the All Tasks, Time, CPU Usage, and Disk I/O reports are now up to date and are complete. 

The only remaining issues are (a) oddities in the sorting by recipe and outcome, and (b) fixing the "page-header div" for the other views, both of which we agreed would be handled separately.

Thanks!
David

> -----Original Message-----
> From: toaster-bounces@yoctoproject.org [mailto:toaster-
> bounces@yoctoproject.org] On Behalf Of Reyna, David
> Sent: Tuesday, February 11, 2014 9:22 AM
> To: Barros Pena, Belen
> Cc: toaster@yoctoproject.org
> Subject: Re: [Toaster] review request for "Disk IO". "CPU Usage", and "Build
> Time" pages
> 
> Hi Belen,
> 
> >* "The fields in the Edit columns menu should be sorted..."
> > I cannot see this change
> 
> Sorry, I missed adding "projecttags.py" to my commit. It is there now.
> 
>   @register.filter(name = 'sortcols')
>   def sortcols(tablecols):
>   - return sorted(tablecols, key = lambda t: t['name'])
>   + return sorted(tablecols, key = lambda t: t['name'].upper())
> 
> > >* "FILTERING In the filter modal dialogs, the 'All' option says..."
> > I cannot see this change either
> 
> Ah, I fixed the title within the dialog box, but not the "all" radio button
> text. I will fix that today.
> 
> > I see the above as well, but the problem I was talking about is a bit
> > different.
> 
> Ok, you have discovered a design flaw. I will file a defect, as it is a
> global problem outside the scope of this commit.
> 
> Since the sort links are currently _always_ the "opposite" sort of any given
> column's sort, when you go from one column to another and then back, the link
> will then dutifully reverse the previous sort, and you will see exactly what
> you observed. You can see that this is the case by selecting the "recipe"
> column sort and hovering on the "CPU Usage" column link, which remained at
> "cpu_usage:+".
> 
> It appears that what we need is that when a column is selected, ALL other
> column sort links should reset so that then when selected they re-assert the
> previous sorting order. Only the current column's link should be the
> "reverse" sort.
> 
> Thanks,
> David
> 
> 
> > -----Original Message-----
> > From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> > Sent: Tuesday, February 11, 2014 4:27 AM
> > To: Reyna, David
> > Cc: toaster@yoctoproject.org
> > Subject: Re: review request for "Disk IO". "CPU Usage", and "Build Time"
> > pages
> >
> >
> >
> > On 10/02/2014 21:55, "Reyna, David" <david.reyna@windriver.com> wrote:
> >
> > >Hi Belen,
> > >
> > >Thank you for the review. The updates are here
> > >"dreyna/time-disk-cpu-view".
> > >
> > >> "Since Ravi worked on the base Tasks table, I am not sure who should be
> > >fixing the page..."
> > >
> > >It should be me. I have taken over this newly merged page, as well as
> > >applying all of the recent style changes.
> > >
> > >
> > >QUESTIONS:
> > >
> > >* "The page-header div has an inline style (margin-top: 40px)."
> > >
> > >I am confused here. I got that style directly from Alex's "build.html"
> > >page line 56, and I used that same value for the approved "recipes.html"
> > >and "bpackage.html" pages.
> > >
> > >I have made this change, but we should then go back and change those
> > >other 3 pages to match.
> >
> > Yes, we should. That's the kind of thing I can do, so I'll go through the
> > templates and make sure they don't have that inline style set.
> >
> > >
> > >* "SORTING: Sorting by recipe and outcome doesn't seem to be working
> > >properly."
> > >
> > >I am not sure what is going on, but I believe that this is a backend
> > >and/or global issue, and should not block this specific commit. I can
> > >open a separate defect to track this issue.
> >
> > Sounds good. Thanks!
> >
> > >
> > >* "SORTING: The default sorting for Time, CPU usage and Disk I/O columns
> > >should be descending"
> > >
> > >Hmm, I do not see the issue. Here is my process.
> > >
> > >  1. Stop the Toaster
> > >  2. Reset the Toaster GUI state for the broswer by:
> > >->   (a) Delete the "localhost" cookies ("_displaycols_*"), and
> > >     (b) Delete the "layers.openembedded.org" key(s) for Toaster
> > >("__utmz" in my case)
> > >  3. Start the Toaster
> > >  4. Select the build, and click "Tasks" link on the left
> > >     I see the "Order" column with a down arrow, and the items from low
> > >to high
> > >  5. Click "Time" link on the left
> > >     I see the "Time" column with an up arrow, and the items from high to
> > >low
> > >  6. Click "CPU Usage" link on the left
> > >     I see the "CPU usage" column with an up arrow, and the items from
> > >high to low
> > >  7. Click "Disk I/O" link on the left
> > >     I see the "Disk I/O (ms)" column with an up arrow, and the items
> > >from high to low
> >
> > I see the above as well, but the problem I was talking about is a bit
> > different. Say that, after step 6, in the CPU usage table, you click on
> > the Task column heading to sort by task. Then you click the CPU column
> > heading again: you will see an arrow down and the items sorted from low to
> > high, but you should see the items sorted from high to low.
> >
> > The default sorting for a column, the one you get on the first click,
> > should be the most useful one. In the case of Time, CPU and Disk I/O, you
> > are likely interested in the most expensive tasks (not the cheap ones),
> > and that's why the default sorting should be from high to low. It is the
> > same with Complete on in the builds table, for example: you are likely to
> > be interested in your latest builds, so the default sorting should show
> > you those at the top.
> >
> > I hope this explains.
> >
> > >
> > >We can have a WebEx session if you still see something different.
> > >
> > >
> > >COMPLETED and PUSHED:
> > >
> > >* "The fields in the Edit columns menu should be sorted..."
> > >
> > >DONE. I have updated the template tag "sortcols" to override the ASCII
> > >sort.
> >
> > I cannot see this change
> >
> > >
> > >* "FILTERING In the filter modal dialogs, the 'All' option says..."
> > >
> > >DONE. I have added the optional "filter_search_display" value to allow
> > >directly control of this customer-facing string, for the same reasons I
> > >added the "object_search_display" string.
> >
> > I cannot see this change either
> >
> > >
> > >* "Search by log and recipe version doesn't seem to be working"
> > >* "When the CPU usage field has no value, the % sign is showing ..."
> > >* "Time value should only display 2 decimal digits..."
> > >* "SEARCH Placeholder search text should say..."
> >
> > But I can see this one! After pulling, this is the latest commit
> >
> > 9cc2a817b08aa0243fbdfcdbd79aec14a23c71eb
> >
> > Is that the right one?
> >
> >
> >
> > >
> > >ALL DONE.
> > >
> > >Thanks,
> > >David
> > >
> > >
> > >-----Original Message-----
> > >From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> > >Sent: Monday, February 10, 2014 4:55 AM
> > >To: Reyna, David; Damian, Alexandru; Ravi Chintakunta
> > >(ravi.chintakunta@timesys.com)
> > >Cc: toaster@yoctoproject.org
> > >Subject: Re: review request for "Disk IO". "CPU Usage", and "Build Time"
> > >pages
> > >
> > >Hi David,
> > >
> > >Thanks for this. Just a few comments on these pages:
> > >
> > >TABLE
> > >
> > >* The fields in the Edit columns menu should be sorted in alphabetical
> > >order (Cache attempt should be before CPU usage)
> > >
> > >* When the CPU usage field has no value, the % sign is showing (it should
> > >not: the cell should be empty)
> > >
> > >* Time value should only display 2 decimal digits. If the value is less
> > >than 0.01, the cell should be empty.
> > >
> > >* The page-header div has an inline style (margin-top: 40px). That should
> > >not be there. Removing it will push the heading up to the same position as
> > >in all other pages.
> > >
> > >
> > >* I know the Cache attempt column is still not working fully as designed
> > >(there is some stuff that needs to be done in the back end). But the
> > >status that is fully working, SSTATE_RESTORED = 3, should be shown as
> > >Succeeded in the table (currently it shows as Restored), in order to match
> > >the copy we¹ll use in the task details pages. I am not sure who should fix
> > >this, though.
> > >
> > >SEARCH
> > >
> > >*  Placeholder search text should say:
> > >
> > >** 'Search tasks' in the tasks table
> > >
> > >** 'Search time data' in the time table
> > >
> > >** 'Search CPU usage data' in the CPU table
> > >
> > >** 'Search disk I/O data' in the disk I/O table
> > >
> > >* Search by log and recipe version doesn't seem to be working
> > >
> > >SORTING
> > >
> > >* Sorting by recipe doesn't seem to be working properly. It seems to be
> > >sorting only a subset of the recipes
> > >
> > >* Sorting by outcome seems to be behaving a bit strangely too: when you
> > >apply reverse sorting, Prebuilt tasks are listed before Covered tasks. The
> > >different outcomes should be listed in alphabetical order. Default sorting
> > >(caret down) should be Cached, Covered, Empty, Failed, Prebuilt and
> > >Succeeded. Reverse sorting (caret up) should be Succeeded, Prebuilt,
> > >Failed, Empty, Covered and Cached.
> > >
> > >* The default sorting for Time, CPU usage and Disk I/O columns should be
> > >descending (highest value on top). Currently, the first time I click on
> > >the heading, it sorts in ascending order.
> > >
> > >FILTERING
> > >
> > >* In the filter modal dialogs, the 'All' option says: 'All buildtime',
> > >'All cpuusage' and 'All diskio'. It should say always: 'All tasks'
> > >
> > >Since Ravi worked on the base Tasks table, I am not sure who should be
> > >fixing the above. I just listed everything I found here :) Any questions /
> > >comments, let me know.
> > >
> > >Cheers
> > >
> > >Belén
> > >
> > >
> > >
> > >
> > >On 10/02/2014 05:40, "Reyna, David" <david.reyna@windriver.com> wrote:
> > >
> > >>Hi all,
> > >>
> > >>* As Belen observed, the three pages "Disk IO". "CPU Usage", and "Build
> > >>Time" are simple variants of Ravi¹s ³All Tasks² page, where the only
> > >>differences are the (a) minimal and default columns, the (b) default
> > >>sort, and (c) the page¹s title and search
> > >> advisory string. Rather than making duplicate (and thus highly redundant
> > >>and fragile) view classes and HTML pages, I have merged all four reports
> > >>into a shared ³tasks_common(request, build_id,
> > >>variant)² view class and HTML page.
> > >>
> > >>* The review branch can be found here: "dreyna/time-disk-cpu-view"
> > >>
> > >>* Because the diff may be hard to read, I am including below a clear-text
> > >>copy of the merged ³tasks² view class below, where I have highlighted the
> > >>net differences in
> > >>green.
> > >>
> > >>* FYI, I have added the variable ³object_search_display². When defined,
> > >>it becomes the default search guidance string in ³basetable_top.html².
> > >>When not defined that display reverts to the normal ³objectname² value.
> > >>This is to support special customer facing
> > >> strings (like ³search disk usage²). I did not want to reuse ³objectname²
> > >>because that variable is overloaded with cookie naming and other internal
> > >>operations, all of which would mostly likely fail with inner spaces.
> > >>
> >
> 
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster

^ permalink raw reply	[flat|nested] 8+ messages in thread