Skip to content

Commit 86122aa

Browse files
data store: wipe workflow data on reload.
* When a workflow is reloaded or restarted (node restart infers reload), there may be objects left behind in the store which have been wiped out in the scheduler by the configuration change. * To handle this, we must wipe all objects stored on the workflow (e.g. TaskProxies, TaskDefs, Jobs, etc), and rebuild from scratch. * Note, we don't need to wipe the data on the Workflow object itself (e.g. port, stateTotals, etc), these things will be refreshed by this or subsequent deltas.
1 parent c95873b commit 86122aa

File tree

7 files changed

+114
-4
lines changed

7 files changed

+114
-4
lines changed

CHANGES.md

+10
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ creating a new release entry be sure to copy & paste the span tag with the
1010
`actions:bind` attribute, which is used by a regex to find the text to be
1111
updated. Only the first match gets replaced, so it's fine to leave the old
1212
ones in. -->
13+
14+
-------------------------------------------------------------------------------
15+
## __cylc-ui-2.2.0 (<span actions:bind='release-date'>Awaiting Release</span>)__
16+
17+
### Fixes
18+
19+
[#1479](https://github.com/cylc/cylc-ui/pull/1479) -
20+
Fix an issue which could result in tasks not being removed from the GUI when
21+
workflows were reloaded or restarted.
22+
1323
-------------------------------------------------------------------------------
1424
## __cylc-ui-2.1.0 (<span actions:bind='release-date'>Released 2023-09-07</span>)__
1525

src/graphql/queries.js

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import gql from 'graphql-tag'
2727
const WORKFLOW_DATA_FRAGMENT = `
2828
fragment WorkflowData on Workflow {
2929
id
30+
reloaded
3031
status
3132
statusMsg
3233
owner

src/services/treeCallback.js

+20-4
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,24 @@ class CylcTreeCallback extends DeltasCallback {
2525
}
2626
}
2727

28-
tearDown (store, errors) {
29-
// never tear down, this callback lives for the live of the UI
28+
before (deltas, store, errors) {
29+
// Wipe all child nodes from a workflow in the data store if a reloaded
30+
// delta is received. Reloaded deltas are sent whenever a workflow is
31+
// restarted or reloaded (note, restarting a workflow implicitly reloads
32+
// it).
33+
//
34+
// When a workflow reloads it is hard to generate the relevant pruned
35+
// and updated deltas to remove any objects which have been wiped out by
36+
// the configuration change, so the easiest solution is to wipe the
37+
// entire tree under the workflow and rebuild from scratch. If we don't
38+
// do this, we can end up with nodes in the store which aren't meant to be
39+
// there and won't get pruned.
40+
if (deltas.updated?.workflow?.reloaded) {
41+
store.commit('workflows/REMOVE_CHILDREN', (deltas.updated.workflow.id))
42+
}
43+
if (deltas.added?.workflow?.reloaded) {
44+
store.commit('workflows/REMOVE_CHILDREN', (deltas.added.workflow.id))
45+
}
3046
}
3147

3248
onAdded (added, store, errors) {
@@ -43,9 +59,9 @@ class CylcTreeCallback extends DeltasCallback {
4359
store.commit('workflows/REMOVE_DELTAS', pruned)
4460
}
4561

46-
// this callback does not need the before and commit methods
47-
before (a, b, c) {}
62+
// this callback does not need the tearDown and commit methods
4863
commit (a, b, c) {}
64+
tearDown (store, errors) {}
4965
}
5066

5167
export default CylcTreeCallback

src/services/workflow.service.js

+1
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ class WorkflowService {
321321
const errors = []
322322

323323
// run the global callback first
324+
globalCallback.before(deltas, store, errors)
324325
globalCallback.onAdded(added, store, errors)
325326
globalCallback.onUpdated(updated, store, errors)
326327
globalCallback.onPruned(pruned, store, errors)

src/views/Graph.vue

+11
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,11 @@ subscription Workflow ($workflowId: ID) {
127127
}
128128
}
129129

130+
fragment workflowdata on workflow {
131+
id
132+
reloaded
133+
}
134+
130135
fragment EdgeData on Edge {
131136
id
132137
source
@@ -154,6 +159,9 @@ fragment JobData on Job {
154159
}
155160

156161
fragment AddedDelta on Added {
162+
workflows {
163+
...workflowdata
164+
}
157165
edges {
158166
...EdgeData
159167
}
@@ -166,6 +174,9 @@ fragment AddedDelta on Added {
166174
}
167175

168176
fragment UpdatedDelta on Updated {
177+
workflows {
178+
...workflowdata
179+
}
169180
edges {
170181
...EdgeData
171182
}

src/views/SimpleTree.vue

+13
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ fragment Deltas on Deltas {
135135
}
136136

137137
fragment AddedDelta on Added {
138+
workflows {
139+
...WorkflowData
140+
}
138141
taskProxies {
139142
...TaskProxyData
140143
}
@@ -144,6 +147,9 @@ fragment AddedDelta on Added {
144147
}
145148

146149
fragment UpdatedDelta on Updated {
150+
workflows {
151+
...WorkflowData
152+
}
147153
taskProxies {
148154
...TaskProxyData
149155
}
@@ -160,6 +166,13 @@ fragment PrunedDelta on Pruned {
160166
jobs
161167
}
162168

169+
# We must always request "id" and "reloaded" for the workflow.
170+
# These are required by the data store.
171+
fragment workflowData on Workflow {
172+
id
173+
reloaded
174+
}
175+
163176
# We must always request the "id" for ALL types.
164177
# The only field this view requires beyond that is the status.
165178
fragment TaskProxyData on TaskProxy {

tests/unit/services/workflow.service.spec.js

+58
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1616
*/
1717

18+
import { createStore } from 'vuex'
19+
import storeOptions from '@/store/options'
20+
1821
import sinon from 'sinon'
1922
import { print } from 'graphql/language'
2023
import gql from 'graphql-tag'
@@ -26,6 +29,7 @@ import WorkflowService from '@/services/workflow.service'
2629
import * as graphqlModule from '@/graphql/index'
2730
import ViewState from '@/model/ViewState.model'
2831
import { TreeCallback, WorkflowCallback } from './testCallback'
32+
import CylcTreeCallback from '@/services/treeCallback'
2933

3034
const sandbox = sinon.createSandbox()
3135

@@ -373,3 +377,57 @@ describe('WorkflowService', () => {
373377
})
374378
})
375379
})
380+
381+
describe('Global Callback', () => {
382+
it('should wipe workflow children on reloaded deltas', () => {
383+
// the callback should wipe workflow children when a "reloaded" delta is
384+
// received - see https://github.com/cylc/cylc-ui/pull/1479
385+
386+
// initiate the store
387+
const errors = {}
388+
const store = createStore(storeOptions)
389+
const callback = new CylcTreeCallback(store, errors)
390+
const cylcTree = store.state.workflows.cylcTree
391+
392+
// send an added delta which adds a workflow with one task
393+
const delta1 = {
394+
id: 123,
395+
added: {
396+
id: 123,
397+
workflow: { id: '~user/foo' },
398+
taskProxies: { id: '~user/foo//1/a' }
399+
}
400+
}
401+
callback.before(delta1, store, errors)
402+
callback.onAdded(delta1.added, store, errors)
403+
404+
// the user/workflow//cycle/task should now be in the store
405+
expect(Object.keys(cylcTree.$index)).to.deep.equal([
406+
'~user',
407+
'~user/foo',
408+
'~user/foo//1',
409+
'~user/foo//1/a',
410+
])
411+
412+
// send a reloaded delta which adds a new task
413+
const delta2 = {
414+
id: 234,
415+
added: {
416+
id: 234,
417+
workflow: { id: '~user/foo', reloaded: true },
418+
taskProxies: { id: '~user/foo//2/b' }
419+
}
420+
}
421+
callback.before(delta2, store, errors)
422+
callback.onUpdated(delta2.added, store, errors)
423+
424+
// the cycle "1" and task "1/a" should be gone from the store
425+
// without the need for an explicit "pruned" delta
426+
expect(Object.keys(cylcTree.$index)).to.deep.equal([
427+
'~user',
428+
'~user/foo',
429+
'~user/foo//2',
430+
'~user/foo//2/b',
431+
])
432+
})
433+
})

0 commit comments

Comments
 (0)