Merge ~ltrager/maas:history_ws_func into maas:master
- Git
- lp:~ltrager/maas
- history_ws_func
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Lee Trager | ||||
Approved revision: | 578be897f9ed0165b10ae545f8ae2a88738d41e6 | ||||
Merge reported by: | MAAS Lander | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~ltrager/maas:history_ws_func | ||||
Merge into: | maas:master | ||||
Diff against target: |
414 lines (+188/-43) 9 files modified
src/maasserver/static/js/angular/controllers/node_results.js (+21/-10) src/maasserver/static/js/angular/controllers/tests/test_node_results.js (+46/-0) src/maasserver/static/js/angular/factories/node_results.js (+10/-1) src/maasserver/static/js/angular/factories/tests/test_node_results.js (+22/-1) src/maasserver/static/partials/script-results-list.html (+6/-3) src/maasserver/websockets/handlers/node_result.py (+21/-13) src/maasserver/websockets/handlers/tests/test_node_result.py (+32/-11) src/metadataserver/models/scriptresult.py (+4/-2) src/metadataserver/models/tests/test_scriptresult.py (+26/-2) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Blake Rouse (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+341493@code.launchpad.net |
Commit message
LP: #1752754 - Request historic results over the websocket instead of always sending them.
Description of the change
- 0430353... by Lee Trager
-
Fix viewing installation results
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b history_ws_func lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 04303530ea5c336
Blake Rouse (blake-rouse) wrote : | # |
See inline comments.
- 09fb7be... by Lee Trager
-
Assign history query to a variable
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b history_ws_func lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED
LOG: http://
COMMIT: 09fb7be87d470eb
- 578be89... by Lee Trager
-
Add query count
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b history_ws_func lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 578be897f9ed016
Lee Trager (ltrager) wrote : | # |
Query count added.
Preview Diff
1 | diff --git a/src/maasserver/static/js/angular/controllers/node_results.js b/src/maasserver/static/js/angular/controllers/node_results.js |
2 | index 101e500..59f3f6f 100644 |
3 | --- a/src/maasserver/static/js/angular/controllers/node_results.js |
4 | +++ b/src/maasserver/static/js/angular/controllers/node_results.js |
5 | @@ -1,4 +1,4 @@ |
6 | -/* Copyright 2017 Canonical Ltd. This software is licensed under the |
7 | +/* Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
8 | * GNU Affero General Public License version 3 (see the file LICENSE). |
9 | * |
10 | * MAAS Node Results Controller |
11 | @@ -135,17 +135,12 @@ angular.module('MAAS').controller('NodeResultsController', [ |
12 | }); |
13 | } else { |
14 | var result = null; |
15 | - var i, j; |
16 | + var i; |
17 | // Find the installation result to be displayed. |
18 | for(i = 0; i < $scope.installation_results.length; i++) { |
19 | - var hlist = $scope.installation_results[i].history_list; |
20 | - for(j = 0; j < hlist.length; j++) { |
21 | - if(hlist[j].id === $scope.logs.option.id) { |
22 | - result = hlist[j]; |
23 | - break; |
24 | - } |
25 | - } |
26 | - if(result) { |
27 | + if($scope.installation_results[i].id === |
28 | + $scope.logs.option.id) { |
29 | + result = $scope.installation_results[i]; |
30 | break; |
31 | } |
32 | } |
33 | @@ -195,6 +190,22 @@ angular.module('MAAS').controller('NodeResultsController', [ |
34 | } |
35 | }; |
36 | |
37 | + $scope.loadHistory = function(result) { |
38 | + result.showing_results = false; |
39 | + // History has already been loaded, no need to request it. |
40 | + if(angular.isArray(result.history_list)) { |
41 | + result.showing_history = true; |
42 | + return; |
43 | + } |
44 | + result.loading_history = true; |
45 | + $scope.nodeResultsManager.get_history(result.id).then( |
46 | + function(history) { |
47 | + result.history_list = history; |
48 | + result.loading_history = false; |
49 | + result.showing_history = true; |
50 | + }); |
51 | + }; |
52 | + |
53 | // Destroy the NodeResultsManager when the scope is destroyed. This is |
54 | // so the client will not recieve any more notifications about results |
55 | // from this node. |
56 | diff --git a/src/maasserver/static/js/angular/controllers/tests/test_node_results.js b/src/maasserver/static/js/angular/controllers/tests/test_node_results.js |
57 | index f2f44d7..fbeb5a1 100644 |
58 | --- a/src/maasserver/static/js/angular/controllers/tests/test_node_results.js |
59 | +++ b/src/maasserver/static/js/angular/controllers/tests/test_node_results.js |
60 | @@ -433,4 +433,50 @@ describe("NodeResultsController", function() { |
61 | "BUG: Unknown log status " + installation_result.status); |
62 | }); |
63 | }); |
64 | + |
65 | + describe("loadHistory", function() { |
66 | + it("loads results", function() { |
67 | + var defer = $q.defer(); |
68 | + var controller = makeController(); |
69 | + var result = { |
70 | + id: makeInteger(0, 100) |
71 | + }; |
72 | + var history_list = [ |
73 | + {id: makeInteger(0, 100)}, |
74 | + {id: makeInteger(0, 100)}, |
75 | + {id: makeInteger(0, 100)} |
76 | + ]; |
77 | + $scope.node = node; |
78 | + $scope.nodeResultsManager = NodeResultsManagerFactory.getManager( |
79 | + $scope.node); |
80 | + spyOn($scope.nodeResultsManager, "get_history").and.returnValue( |
81 | + defer.promise); |
82 | + |
83 | + $scope.loadHistory(result); |
84 | + defer.resolve(history_list); |
85 | + $rootScope.$digest(); |
86 | + |
87 | + expect(result.history_list).toBe(history_list); |
88 | + expect(result.loading_history).toBe(false); |
89 | + expect(result.showing_history).toBe(true); |
90 | + }); |
91 | + |
92 | + it("doesnt reload", function() { |
93 | + var controller = makeController(); |
94 | + var result = { |
95 | + id: makeInteger(0, 100), |
96 | + history_list: [{id: makeInteger(0, 100)}] |
97 | + }; |
98 | + $scope.node = node; |
99 | + $scope.nodeResultsManager = NodeResultsManagerFactory.getManager( |
100 | + $scope.node); |
101 | + spyOn($scope.nodeResultsManager, "get_history"); |
102 | + |
103 | + $scope.loadHistory(result); |
104 | + |
105 | + expect(result.showing_history).toBe(true); |
106 | + expect( |
107 | + $scope.nodeResultsManager.get_history).not.toHaveBeenCalled(); |
108 | + }); |
109 | + }); |
110 | }); |
111 | diff --git a/src/maasserver/static/js/angular/factories/node_results.js b/src/maasserver/static/js/angular/factories/node_results.js |
112 | index 1a0a5de..94a52f7 100644 |
113 | --- a/src/maasserver/static/js/angular/factories/node_results.js |
114 | +++ b/src/maasserver/static/js/angular/factories/node_results.js |
115 | @@ -1,4 +1,4 @@ |
116 | -/* Copyright 2017 Canonical Ltd. This software is licensed under the |
117 | +/* Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
118 | * GNU Affero General Public License version 3 (see the file LICENSE). |
119 | * |
120 | * MAAS NodeResultsManager Manager |
121 | @@ -266,6 +266,15 @@ angular.module('MAAS').factory( |
122 | return RegionConnection.callMethod(method, params); |
123 | }; |
124 | |
125 | + // Get historic data. |
126 | + NodeResultsManager.prototype.get_history = function(script_id) { |
127 | + var method = this._handler + ".get_history"; |
128 | + var params = { |
129 | + id: script_id, |
130 | + }; |
131 | + return RegionConnection.callMethod(method, params); |
132 | + }; |
133 | + |
134 | // Factory that holds all created NodeResultsManagers. |
135 | function NodeResultsManagerFactory() { |
136 | // Holds a list of all NodeResultsManagers that have been created. |
137 | diff --git a/src/maasserver/static/js/angular/factories/tests/test_node_results.js b/src/maasserver/static/js/angular/factories/tests/test_node_results.js |
138 | index 859dda3..2508969 100644 |
139 | --- a/src/maasserver/static/js/angular/factories/tests/test_node_results.js |
140 | +++ b/src/maasserver/static/js/angular/factories/tests/test_node_results.js |
141 | @@ -1,4 +1,4 @@ |
142 | -/* Copyright 2017 Canonical Ltd. This software is licensed under the |
143 | +/* Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
144 | * GNU Affero General Public License version 3 (see the file LICENSE). |
145 | * |
146 | * Unit tests for NodeResultsManagerFactory. |
147 | @@ -532,6 +532,27 @@ describe("NodeResultsManagerFactory", function() { |
148 | }); |
149 | }); |
150 | |
151 | + describe("get_history", function() { |
152 | + |
153 | + it("calls NodeResultHandler.get_history", function(done) { |
154 | + var node = makenode(); |
155 | + var output = [{ |
156 | + id: makeInteger(0, 100), |
157 | + name: makeName("output"), |
158 | + status: makeInteger(0, 100) |
159 | + }]; |
160 | + var id = makeInteger(0, 100); |
161 | + webSocket.returnData.push(makeFakeResponse(output)); |
162 | + NodeResultsManager = NodeResultsManagerFactory.getManager(node); |
163 | + NodeResultsManager.get_history(id).then(function() { |
164 | + var sentObject = angular.fromJson(webSocket.sentData[0]); |
165 | + expect(sentObject.method).toBe("noderesult.get_history"); |
166 | + expect(sentObject.params.id).toEqual(id); |
167 | + done(); |
168 | + }); |
169 | + }); |
170 | + }); |
171 | + |
172 | describe("destroyManager", function() { |
173 | |
174 | it("removes manager from _managers", function() { |
175 | diff --git a/src/maasserver/static/partials/script-results-list.html b/src/maasserver/static/partials/script-results-list.html |
176 | index 4c9ebfc..5ec3585 100644 |
177 | --- a/src/maasserver/static/partials/script-results-list.html |
178 | +++ b/src/maasserver/static/partials/script-results-list.html |
179 | @@ -44,12 +44,11 @@ |
180 | <div class="p-contextual-menu__dropdown" role="menu" data-ng-show="isToggled"> |
181 | <button class="p-contextual-menu__link" aria-label="View metrics" data-ng-if="!result.showing_results" data-ng-click="toggleMenu(); result.showing_history = false; result.showing_results = true">View metrics</button> |
182 | <button class="p-contextual-menu__link" aria-label="Hide metrics" data-ng-if="result.showing_results" data-ng-click="toggleMenu(); result.showing_history = false; result.showing_results = false">Hide metrics</button> |
183 | - <button class="p-contextual-menu__link" aria-label="View previous {$ result.result_section $}" data-ng-if="!result.showing_history" data-ng-click="toggleMenu(); result.showing_results = false; result.showing_history = true">View previous {$ result.result_section $}</button> |
184 | + <button class="p-contextual-menu__link" aria-label="View previous {$ result.result_section $}" data-ng-if="!result.showing_history" data-ng-click="toggleMenu(); loadHistory(result)">View previous {$ result.result_section $}</button> |
185 | <button class="p-contextual-menu__link" aria-label="Hide previous {$ result.result_section $}" data-ng-if="result.showing_history" data-ng-click="toggleMenu(); result.showing_results = false; result.showing_history = false">Hide previous {$ result.result_section $}</button> |
186 | </div> |
187 | </div> |
188 | </td> |
189 | - |
190 | <td class="p-table-expanding__panel col-12" aria-label="results" data-ng-if="result.showing_results && !result.showing_history"> |
191 | <div class="row"> |
192 | <div class="col-12" data-ng-if="result.results.length === 0">No metrics provided</div> |
193 | @@ -64,7 +63,11 @@ |
194 | </dl> |
195 | </div> |
196 | </td> |
197 | - |
198 | + <td class="p-table-expanding__panel col-12" aria-label="loading history" data-ng-if="result.loading_history"> |
199 | + <div class="col-12"> |
200 | + <p class="u-text--loading"><i class="p-icon--spinner u-animation--spin"></i> Loading...</p> |
201 | + </div> |
202 | + </div> |
203 | <td class="p-table-expanding__panel col-12" aria-label="history" data-ng-if="result.showing_history"> |
204 | <div class="row"> |
205 | <div class="col-12"> |
206 | diff --git a/src/maasserver/websockets/handlers/node_result.py b/src/maasserver/websockets/handlers/node_result.py |
207 | index 17bce0d..8704c4f 100644 |
208 | --- a/src/maasserver/websockets/handlers/node_result.py |
209 | +++ b/src/maasserver/websockets/handlers/node_result.py |
210 | @@ -36,6 +36,7 @@ class NodeResultHandler(TimestampedModelHandler): |
211 | 'clear', |
212 | 'get', |
213 | 'get_result_data', |
214 | + 'get_history', |
215 | 'list', |
216 | ] |
217 | listen_channels = ['scriptresult'] |
218 | @@ -92,19 +93,6 @@ class NodeResultHandler(TimestampedModelHandler): |
219 | # Script object. |
220 | data["hardware_type"] = HARDWARE_TYPE.NODE |
221 | data["tags"] = 'commissioning' |
222 | - data["history_list"] = [ |
223 | - { |
224 | - "id": history.id, |
225 | - "updated": dehydrate_datetime(history.updated), |
226 | - "status": history.status, |
227 | - "status_name": history.status_name, |
228 | - "runtime": history.runtime, |
229 | - "starttime": history.starttime, |
230 | - "endtime": history.endtime, |
231 | - "estimated_runtime": history.estimated_runtime, |
232 | - } for history in obj.history.defer( |
233 | - "stdout", "stderr", "output", "result") |
234 | - ] |
235 | try: |
236 | results = obj.read_results() |
237 | except ValidationError as e: |
238 | @@ -217,6 +205,26 @@ class NodeResultHandler(TimestampedModelHandler): |
239 | data = getattr(script_result, data_type) |
240 | return data.decode().strip() |
241 | |
242 | + def get_history(self, params): |
243 | + """Return a list of historic results.""" |
244 | + id = params.get('id') |
245 | + script_result = ScriptResult.objects.filter(id=id).only( |
246 | + 'script_id', 'script_set_id', 'physical_blockdevice_id', |
247 | + 'script_name').first() |
248 | + history_qs = script_result.history.only( |
249 | + 'id', 'updated', 'status', 'started', 'ended', 'script_id', |
250 | + 'script_name', 'script_set_id', 'physical_blockdevice_id') |
251 | + return [{ |
252 | + 'id': history.id, |
253 | + 'updated': dehydrate_datetime(history.updated), |
254 | + 'status': history.status, |
255 | + 'status_name': history.status_name, |
256 | + 'runtime': history.runtime, |
257 | + 'starttime': history.starttime, |
258 | + 'endtime': history.endtime, |
259 | + 'estimated_runtime': history.estimated_runtime, |
260 | + } for history in history_qs] |
261 | + |
262 | def clear(self, params): |
263 | """Clears the current node for events. |
264 | |
265 | diff --git a/src/maasserver/websockets/handlers/tests/test_node_result.py b/src/maasserver/websockets/handlers/tests/test_node_result.py |
266 | index 5b3fc37..a8fe7fe 100644 |
267 | --- a/src/maasserver/websockets/handlers/tests/test_node_result.py |
268 | +++ b/src/maasserver/websockets/handlers/tests/test_node_result.py |
269 | @@ -1,4 +1,4 @@ |
270 | -# Copyright 2017 Canonical Ltd. This software is licensed under the |
271 | +# Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
272 | # GNU Affero General Public License version 3 (see the file LICENSE). |
273 | |
274 | """Tests for `maasserver.websockets.handlers.node_result`""" |
275 | @@ -16,6 +16,7 @@ from maasserver.websockets.base import ( |
276 | HandlerPKError, |
277 | ) |
278 | from maasserver.websockets.handlers.node_result import NodeResultHandler |
279 | +from maastesting.djangotestcase import CountQueries |
280 | from metadataserver.enum import ( |
281 | HARDWARE_TYPE, |
282 | HARDWARE_TYPE_CHOICES, |
283 | @@ -49,16 +50,6 @@ class TestNodeResultHandler(MAASServerTestCase): |
284 | "result_type": script_result.script_set.result_type, |
285 | "hardware_type": script_result.script.hardware_type, |
286 | "tags": ", ".join(script_result.script.tags), |
287 | - "history_list": [{ |
288 | - "id": history.id, |
289 | - "updated": dehydrate_datetime(history.updated), |
290 | - "status": history.status, |
291 | - "status_name": history.status_name, |
292 | - "runtime": history.runtime, |
293 | - "starttime": history.starttime, |
294 | - "endtime": history.endtime, |
295 | - "estimated_runtime": history.estimated_runtime, |
296 | - } for history in script_result.history], |
297 | "results": [{ |
298 | "name": key, |
299 | "title": key, |
300 | @@ -333,6 +324,36 @@ class TestNodeResultHandler(MAASServerTestCase): |
301 | 'data_type': unknown_data_type, |
302 | })) |
303 | |
304 | + def test_get_history(self): |
305 | + user = factory.make_User() |
306 | + handler = NodeResultHandler(user, {}) |
307 | + node = factory.make_Node(owner=user) |
308 | + script = factory.make_Script() |
309 | + script_results = [] |
310 | + for _ in range(10): |
311 | + script_set = factory.make_ScriptSet(node=node) |
312 | + script_results.append(factory.make_ScriptResult( |
313 | + script=script, script_set=script_set, |
314 | + status=SCRIPT_STATUS.PASSED)) |
315 | + latest_script_result = script_results[-1] |
316 | + script_results = sorted( |
317 | + script_results, key=lambda i: i.id, reverse=True) |
318 | + queries = CountQueries() |
319 | + with queries: |
320 | + ret = handler.get_history({'id': latest_script_result.id}) |
321 | + self.assertEqual(4, queries.num_queries) |
322 | + for script_result, out in zip(script_results, ret): |
323 | + self.assertDictEqual({ |
324 | + 'id': script_result.id, |
325 | + 'updated': dehydrate_datetime(script_result.updated), |
326 | + 'status': script_result.status, |
327 | + 'status_name': script_result.status_name, |
328 | + 'runtime': script_result.runtime, |
329 | + 'starttime': script_result.starttime, |
330 | + 'endtime': script_result.endtime, |
331 | + 'estimated_runtime': script_result.estimated_runtime, |
332 | + }, out) |
333 | + |
334 | def test_clear_removes_system_id_from_cache(self): |
335 | user = factory.make_User() |
336 | handler = NodeResultHandler(user, {}) |
337 | diff --git a/src/metadataserver/models/scriptresult.py b/src/metadataserver/models/scriptresult.py |
338 | index e6125cc..d817a0e 100644 |
339 | --- a/src/metadataserver/models/scriptresult.py |
340 | +++ b/src/metadataserver/models/scriptresult.py |
341 | @@ -1,4 +1,4 @@ |
342 | -# Copyright 2017 Canonical Ltd. This software is licensed under the |
343 | +# Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
344 | # GNU Affero General Public License version 3 (see the file LICENSE). |
345 | __all__ = [ |
346 | 'ScriptResult', |
347 | @@ -143,7 +143,9 @@ class ScriptResult(CleanSave, TimestampedModel): |
348 | return self.runtime |
349 | runtime = None |
350 | # Get an estimated runtime from previous runs. |
351 | - for script_result in self.history: |
352 | + for script_result in self.history.only( |
353 | + 'status', 'started', 'ended', 'script_id', 'script_name', |
354 | + 'script_set_id', 'physical_blockdevice_id'): |
355 | # Only look at passed results when calculating an estimated |
356 | # runtime. Failed results may take longer or shorter than |
357 | # average. Don't use self.history.filter for this as the now |
358 | diff --git a/src/metadataserver/models/tests/test_scriptresult.py b/src/metadataserver/models/tests/test_scriptresult.py |
359 | index ba129f3..fedeb6d 100644 |
360 | --- a/src/metadataserver/models/tests/test_scriptresult.py |
361 | +++ b/src/metadataserver/models/tests/test_scriptresult.py |
362 | @@ -1,4 +1,4 @@ |
363 | -# Copyright 2017 Canonical Ltd. This software is licensed under the |
364 | +# Copyright 2017-2018 Canonical Ltd. This software is licensed under the |
365 | # GNU Affero General Public License version 3 (see the file LICENSE). |
366 | |
367 | __all__ = [] |
368 | @@ -19,6 +19,7 @@ from maasserver.models import ( |
369 | from maasserver.testing.factory import factory |
370 | from maasserver.testing.testcase import MAASServerTestCase |
371 | from maasserver.utils.orm import reload_object |
372 | +from maastesting.djangotestcase import CountQueries |
373 | from maastesting.matchers import ( |
374 | DocTestMatches, |
375 | MockCalledOnceWith, |
376 | @@ -30,7 +31,10 @@ from metadataserver.enum import ( |
377 | SCRIPT_STATUS_CHOICES, |
378 | SCRIPT_TYPE, |
379 | ) |
380 | -from metadataserver.models import scriptresult as scriptresult_module |
381 | +from metadataserver.models import ( |
382 | + ScriptResult, |
383 | + scriptresult as scriptresult_module, |
384 | +) |
385 | from provisioningserver.events import EVENT_TYPES |
386 | import yaml |
387 | |
388 | @@ -600,6 +604,26 @@ class TestScriptResult(MAASServerTestCase): |
389 | script_result = script_results[-1] |
390 | self.assertItemsEqual(script_results, script_result.history) |
391 | |
392 | + def test_history_query_count(self): |
393 | + script_name = factory.make_name('script_name') |
394 | + script_set = factory.make_ScriptSet() |
395 | + script_results = [ |
396 | + factory.make_ScriptResult( |
397 | + script_name=script_name, script_set=script_set) |
398 | + for _ in range(10) |
399 | + ] |
400 | + queries_one = CountQueries() |
401 | + script_result_one = ScriptResult.objects.get(id=script_results[0].id) |
402 | + with queries_one: |
403 | + script_result_one.history |
404 | + queries_many = CountQueries() |
405 | + script_result_many = ScriptResult.objects.get( |
406 | + id=script_results[-1].id) |
407 | + with queries_many: |
408 | + script_result_many.history |
409 | + self.assertEquals(1, queries_one.num_queries) |
410 | + self.assertEquals(1, queries_many.num_queries) |
411 | + |
412 | def test_history_storage_device(self): |
413 | # Regression test for LP: #1721524 |
414 | script = factory.make_Script() |
UNIT TESTS
-b history_ws_func lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED maas-ci- jenkins. internal: 8080/job/ maas/job/ branch- tester/ 1943/console 3f141354be70d85 1ef4493e11
LOG: http://
COMMIT: 2b6a0c90803e1f7