From 9839061745cc44f95e09cbc6f2984d66dd62a1b3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 17:48:52 -0700 Subject: [PATCH 1/8] Sort allocation by status type, not simply alphanumeric --- ui/app/models/allocation.js | 11 +++++++++++ ui/app/templates/jobs/job/task-group.hbs | 2 +- ui/app/templates/nodes/node.hbs | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ui/app/models/allocation.js b/ui/app/models/allocation.js index e77183202..3862c358a 100644 --- a/ui/app/models/allocation.js +++ b/ui/app/models/allocation.js @@ -10,6 +10,14 @@ import shortUUIDProperty from '../utils/properties/short-uuid'; const { computed, RSVP } = Ember; +const STATUS_ORDER = { + pending: 1, + running: 2, + complete: 3, + failed: 4, + lost: 5, +}; + export default Model.extend({ shortId: shortUUIDProperty('id'), job: belongsTo('job'), @@ -24,6 +32,9 @@ export default Model.extend({ clientStatus: attr('string'), desiredStatus: attr('string'), + statusIndex: computed('clientStatus', function() { + return STATUS_ORDER[this.get('clientStatus')] || 100; + }), taskGroup: computed('taskGroupName', 'job.taskGroups.[]', function() { const taskGroups = this.get('job.taskGroups'); diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index bb4a0ed93..c8c1ea103 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -68,7 +68,7 @@ {{#t.head}} {{#t.sort-by prop="shortId"}}ID{{/t.sort-by}} {{#t.sort-by prop="name"}}Name{{/t.sort-by}} - {{#t.sort-by prop="clientStatus"}}Status{{/t.sort-by}} + {{#t.sort-by prop="statusIndex"}}Status{{/t.sort-by}} {{#t.sort-by prop="node.shortId"}}Node{{/t.sort-by}} CPU Memory diff --git a/ui/app/templates/nodes/node.hbs b/ui/app/templates/nodes/node.hbs index 0fb81d11e..dc0110690 100644 --- a/ui/app/templates/nodes/node.hbs +++ b/ui/app/templates/nodes/node.hbs @@ -41,7 +41,7 @@ {{#t.head}} {{#t.sort-by prop="shortId"}}ID{{/t.sort-by}} {{#t.sort-by prop="name"}}Name{{/t.sort-by}} - {{#t.sort-by prop="clientStatus"}}Status{{/t.sort-by}} + {{#t.sort-by prop="statusIndex"}}Status{{/t.sort-by}} {{#t.sort-by prop="job.name"}}Job{{/t.sort-by}} CPU Memory From 7007d04deeed5ef0e6ab4096a43839ab9801654f Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 17:49:59 -0700 Subject: [PATCH 2/8] Add ModifyIndex as a sortable column for alloc tables --- ui/app/templates/components/allocation-row.hbs | 1 + ui/app/templates/jobs/job/task-group.hbs | 1 + ui/app/templates/nodes/node.hbs | 1 + 3 files changed, 3 insertions(+) diff --git a/ui/app/templates/components/allocation-row.hbs b/ui/app/templates/components/allocation-row.hbs index 9aedfed02..f440f074c 100644 --- a/ui/app/templates/components/allocation-row.hbs +++ b/ui/app/templates/components/allocation-row.hbs @@ -3,6 +3,7 @@ {{allocation.shortId}} +{{allocation.modifyIndex}} {{allocation.name}} {{allocation.clientStatus}} diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index c8c1ea103..6d010d199 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -67,6 +67,7 @@ sortDescending=sortDescending as |t|}} {{#t.head}} {{#t.sort-by prop="shortId"}}ID{{/t.sort-by}} + {{#t.sort-by prop="modifyIndex" title="Modify Index"}}Modified{{/t.sort-by}} {{#t.sort-by prop="name"}}Name{{/t.sort-by}} {{#t.sort-by prop="statusIndex"}}Status{{/t.sort-by}} {{#t.sort-by prop="node.shortId"}}Node{{/t.sort-by}} diff --git a/ui/app/templates/nodes/node.hbs b/ui/app/templates/nodes/node.hbs index dc0110690..5b6ff2fea 100644 --- a/ui/app/templates/nodes/node.hbs +++ b/ui/app/templates/nodes/node.hbs @@ -40,6 +40,7 @@ class="allocations with-foot" as |t|}} {{#t.head}} {{#t.sort-by prop="shortId"}}ID{{/t.sort-by}} + {{#t.sort-by prop="modifyIndex" title="Modify Index"}}Modified{{/t.sort-by}} {{#t.sort-by prop="name"}}Name{{/t.sort-by}} {{#t.sort-by prop="statusIndex"}}Status{{/t.sort-by}} {{#t.sort-by prop="job.name"}}Job{{/t.sort-by}} From 6ee142ace14117dc7ecc8837461094a945c6ff44 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 17:51:00 -0700 Subject: [PATCH 3/8] Make ModifyIndex the default sort property for alloc tables --- ui/app/components/list-table/sort-by.js | 2 ++ ui/app/controllers/jobs/job/task-group.js | 4 ++-- ui/mirage/factories/allocation.js | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ui/app/components/list-table/sort-by.js b/ui/app/components/list-table/sort-by.js index eac7deae0..9edcb61e7 100644 --- a/ui/app/components/list-table/sort-by.js +++ b/ui/app/components/list-table/sort-by.js @@ -5,6 +5,8 @@ const { Component, computed } = Ember; export default Component.extend({ tagName: 'th', + attributeBindings: ['title'], + // The prop that the table is currently sorted by currentProp: '', diff --git a/ui/app/controllers/jobs/job/task-group.js b/ui/app/controllers/jobs/job/task-group.js index 89974810b..1b408a22c 100644 --- a/ui/app/controllers/jobs/job/task-group.js +++ b/ui/app/controllers/jobs/job/task-group.js @@ -17,8 +17,8 @@ export default Controller.extend(Sortable, Searchable, { currentPage: 1, pageSize: 10, - sortProperty: 'name', - sortDescending: false, + sortProperty: 'modifyIndex', + sortDescending: true, searchProps: computed(() => ['id', 'name']), diff --git a/ui/mirage/factories/allocation.js b/ui/mirage/factories/allocation.js index 8ae323ef7..097ac61d0 100644 --- a/ui/mirage/factories/allocation.js +++ b/ui/mirage/factories/allocation.js @@ -9,6 +9,8 @@ const DESIRED_STATUSES = ['run', 'stop', 'evict']; export default Factory.extend({ id: i => (i >= 100 ? `${UUIDS[i % 100]}-${i}` : UUIDS[i]), + modifyIndex: () => faker.random.number({ min: 10, max: 2000 }), + clientStatus: faker.list.random(...CLIENT_STATUSES), desiredStatus: faker.list.random(...DESIRED_STATUSES), From 68e444d660a5bc6d12da3149de6811f4f6034ed4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 17:51:40 -0700 Subject: [PATCH 4/8] Fix an issue where allocation rows can have a state change in the same frame as a render --- ui/app/components/allocation-row.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/ui/app/components/allocation-row.js b/ui/app/components/allocation-row.js index 129f096eb..9606fae8d 100644 --- a/ui/app/components/allocation-row.js +++ b/ui/app/components/allocation-row.js @@ -1,7 +1,7 @@ import Ember from 'ember'; import { lazyClick } from '../helpers/lazy-click'; -const { Component, inject } = Ember; +const { Component, inject, run } = Ember; export default Component.extend({ store: inject.service(), @@ -46,16 +46,18 @@ export default Component.extend({ // workaround is to persist the jobID as a string on the allocation // and manually re-link the two records here. - const allocation = this.get('allocation'); - const job = this.get('store').peekRecord('job', allocation.get('originalJobId')); - if (job) { - allocation.set('job', job); - } else { - this.get('store') - .findRecord('job', allocation.get('originalJobId')) - .then(job => { - allocation.set('job', job); - }); - } + run.next(() => { + const allocation = this.get('allocation'); + const job = this.get('store').peekRecord('job', allocation.get('originalJobId')); + if (job) { + allocation.set('job', job); + } else { + this.get('store') + .findRecord('job', allocation.get('originalJobId')) + .then(job => { + allocation.set('job', job); + }); + } + }); }, }); From 2bf8945e02a5cb9b5648688b7acbb27243980a2a Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 17:52:31 -0700 Subject: [PATCH 5/8] Use the right footer pagination styles on task group page --- ui/app/templates/jobs/job/task-group.hbs | 26 +++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/ui/app/templates/jobs/job/task-group.hbs b/ui/app/templates/jobs/job/task-group.hbs index 6d010d199..8f86fb8a2 100644 --- a/ui/app/templates/jobs/job/task-group.hbs +++ b/ui/app/templates/jobs/job/task-group.hbs @@ -64,7 +64,8 @@ {{#list-table source=p.list sortProperty=sortProperty - sortDescending=sortDescending as |t|}} + sortDescending=sortDescending + class="with-foot" as |t|}} {{#t.head}} {{#t.sort-by prop="shortId"}}ID{{/t.sort-by}} {{#t.sort-by prop="modifyIndex" title="Modify Index"}}Modified{{/t.sort-by}} @@ -78,19 +79,16 @@ {{allocation-row allocation=row.model context="job" onClick=(action "gotoAllocation" row.model)}} {{/t.body}} {{/list-table}} - +
+ +
{{else}}
From 106967bd62579fa9cab92fcb7a22877c352c3dff Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 19:18:04 -0700 Subject: [PATCH 6/8] Don't double render, also don't render infinitely See https://github.com/emberjs/ember.js/issues/13948 --- ui/app/components/allocation-row.js | 34 +++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/ui/app/components/allocation-row.js b/ui/app/components/allocation-row.js index 9606fae8d..a258f5b24 100644 --- a/ui/app/components/allocation-row.js +++ b/ui/app/components/allocation-row.js @@ -45,19 +45,25 @@ export default Component.extend({ // being resolved through the store (store.findAll('job')). The // workaround is to persist the jobID as a string on the allocation // and manually re-link the two records here. - - run.next(() => { - const allocation = this.get('allocation'); - const job = this.get('store').peekRecord('job', allocation.get('originalJobId')); - if (job) { - allocation.set('job', job); - } else { - this.get('store') - .findRecord('job', allocation.get('originalJobId')) - .then(job => { - allocation.set('job', job); - }); - } - }); + run.scheduleOnce('afterRender', this, qualifyJob); }, }); + +function qualifyJob() { + const allocation = this.get('allocation'); + if (allocation.get('originalJobId')) { + const job = this.get('store').peekRecord('job', allocation.get('originalJobId')); + if (job) { + allocation.setProperties({ + job, + originalJobId: null, + }); + } else { + this.get('store') + .findRecord('job', allocation.get('originalJobId')) + .then(job => { + allocation.set('job', job); + }); + } + } +} From 4382e696e7e64e8017d8dc0d5e4f301c30a40e65 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 19:18:49 -0700 Subject: [PATCH 7/8] Always provide a valid value for transform --- ui/app/components/distribution-bar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/components/distribution-bar.js b/ui/app/components/distribution-bar.js index b8a043b72..105273df6 100644 --- a/ui/app/components/distribution-bar.js +++ b/ui/app/components/distribution-bar.js @@ -125,7 +125,7 @@ export default Component.extend(WindowResizable, { .attr('y', () => isNarrow ? '50%' : 0) .attr('clip-path', `url(#${this.get('maskId')})`) .attr('height', () => isNarrow ? '6px' : '100%') - .attr('transform', () => isNarrow && 'translate(0, -3)') + .attr('transform', () => isNarrow ? 'translate(0, -3)' : '') .merge(layers) .attr('class', (d, i) => `bar layer-${i}`) .transition() From 06f781db8d3ca8990fb2a82fec081b59d1c7ca86 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 17 Oct 2017 19:19:02 -0700 Subject: [PATCH 8/8] Update tests for the ModifyIndex column and sort change --- ui/tests/acceptance/client-detail-test.js | 28 +++--- ui/tests/acceptance/task-group-detail-test.js | 86 +++++++++++-------- 2 files changed, 66 insertions(+), 48 deletions(-) diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index 59c80a0fb..da37a70d8 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -127,12 +127,20 @@ test('each allocation should have high-level details for the allocation', functi .find('td:eq(1)') .text() .trim(), + allocation.modifyIndex, + 'Allocation modify index' + ); + assert.equal( + allocationRow + .find('td:eq(2)') + .text() + .trim(), allocation.name, 'Allocation name' ); assert.equal( allocationRow - .find('td:eq(2)') + .find('td:eq(3)') .text() .trim(), allocation.clientStatus, @@ -140,41 +148,41 @@ test('each allocation should have high-level details for the allocation', functi ); assert.ok( allocationRow - .find('td:eq(3)') + .find('td:eq(4)') .text() .includes(server.db.jobs.find(allocation.jobId).name), 'Job name' ); assert.ok( allocationRow - .find('td:eq(3) .is-faded') + .find('td:eq(4) .is-faded') .text() .includes(allocation.taskGroup), 'Task group name' ); assert.equal( allocationRow - .find('td:eq(4)') + .find('td:eq(5)') .text() .trim(), allocStats.resourceUsage.CpuStats.Percent, 'CPU %' ); assert.equal( - allocationRow.find('td:eq(4) .tooltip').attr('aria-label'), + allocationRow.find('td:eq(5) .tooltip').attr('aria-label'), `${Math.floor(allocStats.resourceUsage.CpuStats.TotalTicks)} / ${cpuUsed} MHz`, 'Detailed CPU information is in a tooltip' ); assert.equal( allocationRow - .find('td:eq(5)') + .find('td:eq(6)') .text() .trim(), allocStats.resourceUsage.MemoryStats.RSS / 1024 / 1024 / memoryUsed, 'Memory used' ); assert.equal( - allocationRow.find('td:eq(5) .tooltip').attr('aria-label'), + allocationRow.find('td:eq(6) .tooltip').attr('aria-label'), `${formatBytes([allocStats.resourceUsage.MemoryStats.RSS])} / ${memoryUsed} MiB`, 'Detailed memory information is in a tooltip' ); @@ -209,14 +217,14 @@ test('each allocation should show job information even if the job is incomplete assert.ok( allocationRow - .find('td:eq(3)') + .find('td:eq(4)') .text() .includes(server.db.jobs.find(allocation.jobId).name), 'Job name' ); assert.ok( allocationRow - .find('td:eq(3) .is-faded') + .find('td:eq(4) .is-faded') .text() .includes(allocation.taskGroup), 'Task group name' @@ -252,7 +260,7 @@ test('each allocation should link to the job the allocation belongs to', functio const job = server.db.jobs.find(allocation.jobId); andThen(() => { - click($('.allocations tbody tr:eq(0) td:eq(3) a').get(0)); + click($('.allocations tbody tr:eq(0) td:eq(4) a').get(0)); }); andThen(() => { diff --git a/ui/tests/acceptance/task-group-detail-test.js b/ui/tests/acceptance/task-group-detail-test.js index 8d83963e8..c1f434f74 100644 --- a/ui/tests/acceptance/task-group-detail-test.js +++ b/ui/tests/acceptance/task-group-detail-test.js @@ -140,43 +140,53 @@ test('/jobs/:id/:task-group should list one page of allocations for the task gro }); test('each allocation should show basic information about the allocation', function(assert) { - const allocation = allocations.sortBy('name')[0]; + const allocation = allocations.sortBy('modifyIndex').reverse()[0]; const allocationRow = $(findAll('.allocations tbody tr')[0]); - assert.equal( - allocationRow - .find('td:eq(0)') - .text() - .trim(), - allocation.id.split('-')[0], - 'Allocation short id' - ); - assert.equal( - allocationRow - .find('td:eq(1)') - .text() - .trim(), - allocation.name, - 'Allocation name' - ); - assert.equal( - allocationRow - .find('td:eq(2)') - .text() - .trim(), - allocation.clientStatus, - 'Client status' - ); - assert.equal( - allocationRow - .find('td:eq(3)') - .text() - .trim(), - server.db.nodes.find(allocation.nodeId).id.split('-')[0], - 'Node ID' - ); + andThen(() => { + assert.equal( + allocationRow + .find('td:eq(0)') + .text() + .trim(), + allocation.id.split('-')[0], + 'Allocation short id' + ); + assert.equal( + allocationRow + .find('td:eq(1)') + .text() + .trim(), + allocation.modifyIndex, + 'Allocation modify index' + ); + assert.equal( + allocationRow + .find('td:eq(2)') + .text() + .trim(), + allocation.name, + 'Allocation name' + ); + assert.equal( + allocationRow + .find('td:eq(3)') + .text() + .trim(), + allocation.clientStatus, + 'Client status' + ); + assert.equal( + allocationRow + .find('td:eq(4)') + .text() + .trim(), + server.db.nodes.find(allocation.nodeId).id.split('-')[0], + 'Node ID' + ); + }); - click(allocationRow.find('td:eq(3) a').get(0)); + click(allocationRow.find('td:eq(4) a').get(0)); andThen(() => { assert.equal(currentURL(), `/nodes/${allocation.nodeId}`, 'Node links to node page'); @@ -196,7 +206,7 @@ test('each allocation should show stats about the allocation, retrieved directly assert.equal( allocationRow - .find('td:eq(4)') + .find('td:eq(5)') .text() .trim(), allocStats.resourceUsage.CpuStats.Percent, @@ -204,14 +214,14 @@ test('each allocation should show stats about the allocation, retrieved directly ); assert.equal( - allocationRow.find('td:eq(4) .tooltip').attr('aria-label'), + allocationRow.find('td:eq(5) .tooltip').attr('aria-label'), `${Math.floor(allocStats.resourceUsage.CpuStats.TotalTicks)} / ${cpuUsed} MHz`, 'Detailed CPU information is in a tooltip' ); assert.equal( allocationRow - .find('td:eq(5)') + .find('td:eq(6)') .text() .trim(), allocStats.resourceUsage.MemoryStats.RSS / 1024 / 1024 / memoryUsed, @@ -219,7 +229,7 @@ test('each allocation should show stats about the allocation, retrieved directly ); assert.equal( - allocationRow.find('td:eq(5) .tooltip').attr('aria-label'), + allocationRow.find('td:eq(6) .tooltip').attr('aria-label'), `${formatBytes([allocStats.resourceUsage.MemoryStats.RSS])} / ${memoryUsed} MiB`, 'Detailed memory information is in a tooltip' );