Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2035793004:
[Findit] Updating try job dashboard to display in queue and execution times (Closed)
Description[Findit] Updating try job dashboard to display in queue and execution times
Example Updated UI:
https://lijeffrey-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?start_date=2016-05-01&end_date=2016-06-03
BUG=616847
Committed: https://chromium.googlesource.com/infra/infra/+/cdb23fed7e365a36588426b0fb89201d41346298
Patch Set 1 #
Total comments: 9
Patch Set 2 : Addressing comments #
Total comments: 2
Patch Set 3 : Addressing comments #Patch Set 4 : Fixing unit tests #
Messages
Total messages: 23 (10 generated)
Description was changed from ========== [Findit] Updating try job dashboard to display in queue and execution times BUG= ========== to ========== [Findit] Updating try job dashboard to display in queue and execution times Example Updated UI: https://lijeffrey-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?s... BUG=616847 ==========
lijeffrey@chromium.org changed reviewers: + chanli@chromium.org, stgao@chromium.org
ptal
lgtm with nits. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:26: return timedelta(0) Should we do "N/A" instead if any time is None? https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:99: try_job_display_data['in_queue_time'] = _FormatDuration( nit: "pending time"? Same for UI.
https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:113: 'try_jobs_in_progress': try_jobs_in_progress, Sorted by reverse of start time? Same for the other two categories. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/templates/... File appengine/findit/templates/try_job_dashboard.html (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/templates/... appengine/findit/templates/try_job_dashboard.html:56: <th>Start Time</th> Should we have pending time here too?
lgtm
comments addressed, ptal. I've deployed another test version so the link from last time should still be valid if there are any other design requests https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:26: return timedelta(0) On 2016/06/02 22:13:57, stgao wrote: > Should we do "N/A" instead if any time is None? Done. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:99: try_job_display_data['in_queue_time'] = _FormatDuration( On 2016/06/02 22:13:57, stgao wrote: > nit: "pending time"? Same for UI. Done. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:113: 'try_jobs_in_progress': try_jobs_in_progress, On 2016/06/02 22:16:33, stgao wrote: > Sorted by reverse of start time? > > Same for the other two categories. Since start time may not always be available, we can sort by request time. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/templates/... File appengine/findit/templates/try_job_dashboard.html (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/templates/... appengine/findit/templates/try_job_dashboard.html:56: <th>Start Time</th> On 2016/06/02 22:16:33, stgao wrote: > Should we have pending time here too? Done.
lgtm with nits. Once fixed, feel free to CQ. https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/1/appengine/findit/handlers/t... appengine/findit/handlers/try_job_dashboard.py:113: 'try_jobs_in_progress': try_jobs_in_progress, On 2016/06/03 03:18:27, lijeffrey wrote: > On 2016/06/02 22:16:33, stgao wrote: > > Sorted by reverse of start time? > > > > Same for the other two categories. > > Since start time may not always be available, we can sort by request time. sg. But why #79 still sort by start_time? https://codereview.chromium.org/2035793004/diff/20001/appengine/findit/handle... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/20001/appengine/findit/handle... appengine/findit/handlers/try_job_dashboard.py:28: if not start_time or not end_time: Should this be moved to #24 above and get rid of #34-35?
https://codereview.chromium.org/2035793004/diff/20001/appengine/findit/handle... File appengine/findit/handlers/try_job_dashboard.py (right): https://codereview.chromium.org/2035793004/diff/20001/appengine/findit/handle... appengine/findit/handlers/try_job_dashboard.py:28: if not start_time or not end_time: On 2016/06/03 06:36:04, stgao wrote: > Should this be moved to #24 above and get rid of #34-35? Done.
The CQ bit was checked by lijeffrey@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chanli@chromium.org, stgao@chromium.org Link to the patchset: https://codereview.chromium.org/2035793004/#ps40001 (title: "Addressing comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035793004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Precise...)
The CQ bit was checked by lijeffrey@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035793004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Trusty 64 Tester on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Trusty%...)
The CQ bit was checked by lijeffrey@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chanli@chromium.org, stgao@chromium.org Link to the patchset: https://codereview.chromium.org/2035793004/#ps60001 (title: "Fixing unit tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2035793004/60001
Message was sent while issue was closed.
Description was changed from ========== [Findit] Updating try job dashboard to display in queue and execution times Example Updated UI: https://lijeffrey-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?s... BUG=616847 ========== to ========== [Findit] Updating try job dashboard to display in queue and execution times Example Updated UI: https://lijeffrey-dot-findit-for-me.appspot.com/waterfall/try-job-dashboard?s... BUG=616847 Committed: https://chromium.googlesource.com/infra/infra/+/cdb23fed7e365a36588426b0fb892... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/infra/infra/+/cdb23fed7e365a36588426b0fb892... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
