From cf4347706be7b8af53a9000f7fa25f0b107cb843 Mon Sep 17 00:00:00 2001 From: David Sutherland Date: Thu, 31 Mar 2022 15:07:00 +1300 Subject: [PATCH 1/4] fix graphql node state and remove ghosts filters --- cylc/flow/network/resolvers.py | 15 ++++++++------- cylc/flow/network/schema.py | 25 ------------------------- tests/integration/test_resolvers.py | 3 --- 3 files changed, 8 insertions(+), 35 deletions(-) diff --git a/cylc/flow/network/resolvers.py b/cylc/flow/network/resolvers.py index cef6b784b8d..ef176fc7d76 100644 --- a/cylc/flow/network/resolvers.py +++ b/cylc/flow/network/resolvers.py @@ -194,7 +194,10 @@ def node_ids_filter(tokens, state, items) -> bool: ) # match cycle/task/job state and ( - not get_state_from_selectors(item) + not ( + state + and get_state_from_selectors(item) + ) or get_state_from_selectors(item) == state ) ) @@ -219,12 +222,10 @@ def node_filter(node, node_type, args): state = getattr(node, 'state', None) return ( ( - args.get('ghosts') - or state - or node_type in DEF_TYPES - ) - and ( - not args.get('states') + not ( + state + and args.get('states') + ) or state in args['states'] ) and not ( diff --git a/cylc/flow/network/schema.py b/cylc/flow/network/schema.py index 4fccacab196..482293c3f98 100644 --- a/cylc/flow/network/schema.py +++ b/cylc/flow/network/schema.py @@ -148,7 +148,6 @@ class SortArgs(InputObjectType): reverse = Boolean(default_value=False) -GHOSTS_DEFAULT = Boolean(default_value=False) STRIP_NULL_DEFAULT = Argument( Boolean, description="A flag that when enabled strips out those fields " "not set in the protobuf object. And when this flag " @@ -633,7 +632,6 @@ class Meta: lambda: TaskProxy, description="""Task cycle instances.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -642,7 +640,6 @@ class Meta: lambda: FamilyProxy, description="""Family cycle instances.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -665,7 +662,6 @@ class Meta: nodes_edges = Field( lambda: NodesEdges, args=NODES_EDGES_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -757,7 +753,6 @@ class Meta: lambda: TaskProxy, description="""Associated cycle point proxies""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -912,7 +907,6 @@ class Meta: lambda: FamilyProxy, description="""Task parents.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -921,7 +915,6 @@ class Meta: lambda: FamilyProxy, description="""Task first parent.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -930,7 +923,6 @@ class Meta: lambda: FamilyProxy, description="""First parent ancestors.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -948,7 +940,6 @@ class Meta: lambda: FamilyProxy, description="""Associated cycle point proxies""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -1014,7 +1005,6 @@ class Meta: TaskProxy, description="""Descendant task proxies.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -1023,7 +1013,6 @@ class Meta: lambda: FamilyProxy, description="""Descendant family proxies.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -1032,7 +1021,6 @@ class Meta: lambda: FamilyProxy, description="""Task first parent.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -1041,7 +1029,6 @@ class Meta: lambda: FamilyProxy, description="""First parent ancestors.""", args=PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, delta_store=DELTA_STORE_DEFAULT, delta_type=DELTA_TYPE_DEFAULT, @@ -1152,7 +1139,6 @@ class Meta: TaskProxy, description=TaskProxy._meta.description, args=ALL_PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, resolver=get_nodes_all) family = Field( @@ -1177,7 +1163,6 @@ class Meta: FamilyProxy, description=FamilyProxy._meta.description, args=ALL_PROXY_ARGS, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, resolver=get_nodes_all) edges = graphene.List( @@ -1190,7 +1175,6 @@ class Meta: NodesEdges, description=NodesEdges._meta.description, args=NODES_EDGES_ARGS_ALL, - ghosts=GHOSTS_DEFAULT, strip_null=STRIP_NULL_DEFAULT, resolver=get_nodes_edges) @@ -1943,7 +1927,6 @@ class Delta(Interface): FamilyProxy, description="""Family cycle instances.""", args=PROXY_ARGS, - ghosts=Boolean(default_value=True), strip_null=Boolean(), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -1971,7 +1954,6 @@ class Delta(Interface): TaskProxy, description="""Task cycle instances.""", args=PROXY_ARGS, - ghosts=Boolean(default_value=True), strip_null=Boolean(), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -2020,7 +2002,6 @@ class Meta: FamilyProxy, description="""Family cycle instances.""", args=PROXY_ARGS, - ghosts=Boolean(default_value=False), strip_null=Boolean(), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_UPDATED), @@ -2048,7 +2029,6 @@ class Meta: TaskProxy, description="""Task cycle instances.""", args=PROXY_ARGS, - ghosts=Boolean(default_value=False), strip_null=Boolean(), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_UPDATED), @@ -2172,7 +2152,6 @@ class Meta: TaskProxy, description=TaskProxy._meta.description, id=ID(required=True), - ghosts=Boolean(default_value=True), strip_null=Boolean(default_value=True), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -2184,7 +2163,6 @@ class Meta: TaskProxy, description=TaskProxy._meta.description, args=ALL_PROXY_ARGS, - ghosts=Boolean(default_value=True), strip_null=Boolean(default_value=True), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -2218,7 +2196,6 @@ class Meta: FamilyProxy, description=FamilyProxy._meta.description, id=ID(required=True), - ghosts=Boolean(default_value=True), strip_null=Boolean(default_value=True), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -2230,7 +2207,6 @@ class Meta: FamilyProxy, description=FamilyProxy._meta.description, args=ALL_PROXY_ARGS, - ghosts=Boolean(default_value=True), strip_null=Boolean(default_value=True), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), @@ -2253,7 +2229,6 @@ class Meta: NodesEdges, description=NodesEdges._meta.description, args=NODES_EDGES_ARGS_ALL, - ghosts=Boolean(default_value=True), strip_null=Boolean(default_value=True), delta_store=Boolean(default_value=True), delta_type=String(default_value=DELTA_ADDED), diff --git a/tests/integration/test_resolvers.py b/tests/integration/test_resolvers.py index 9be51209571..00fa855acc8 100644 --- a/tests/integration/test_resolvers.py +++ b/tests/integration/test_resolvers.py @@ -35,7 +35,6 @@ def flow_args(): @pytest.fixture def node_args(): return { - 'ghosts': False, 'workflows': [], 'exworkflows': [], 'ids': [], @@ -114,7 +113,6 @@ async def test_get_nodes_all(mock_flow, node_args): node_args['states'].append('failed') nodes = await mock_flow.resolvers.get_nodes_all(TASK_PROXIES, node_args) assert len(nodes) == 0 - node_args['ghosts'] = True node_args['states'] = [] node_args['ids'].append(Tokens(mock_flow.node_ids[0])) nodes = [ @@ -136,7 +134,6 @@ async def test_get_nodes_by_ids(mock_flow, node_args): nodes = await mock_flow.resolvers.get_nodes_by_ids(TASK_PROXIES, node_args) assert len(nodes) == 0 - node_args['ghosts'] = True node_args['native_ids'] = mock_flow.node_ids nodes = [ n From 4cece6e5e9aea4017e91ed6dd3860a418a78020f Mon Sep 17 00:00:00 2001 From: David Sutherland Date: Fri, 1 Apr 2022 12:38:12 +1300 Subject: [PATCH 2/4] node state data-store lookup --- cylc/flow/network/resolvers.py | 56 ++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/cylc/flow/network/resolvers.py b/cylc/flow/network/resolvers.py index ef176fc7d76..4ccec9455c6 100644 --- a/cylc/flow/network/resolvers.py +++ b/cylc/flow/network/resolvers.py @@ -195,7 +195,7 @@ def node_ids_filter(tokens, state, items) -> bool: # match cycle/task/job state and ( not ( - state + state is not None and get_state_from_selectors(item) ) or get_state_from_selectors(item) == state @@ -205,7 +205,7 @@ def node_ids_filter(tokens, state, items) -> bool: ) -def node_filter(node, node_type, args): +def node_filter(node, node_type, args, state): """Filter nodes based on attribute arguments""" tokens: Tokens if node_type in DEF_TYPES: @@ -219,18 +219,17 @@ def node_filter(node, node_type, args): else: # live objects can be represented by a universal ID tokens = Tokens(node.id) - state = getattr(node, 'state', None) return ( ( - not ( - state - and args.get('states') + ( + state is None + or not args.get('states') ) or state in args['states'] ) - and not ( - args.get('exstates') - and state in args['exstates'] + and ( + not args.get('exstates') + or state not in args['exstates'] ) and ( args.get('is_held') is None @@ -253,9 +252,9 @@ def node_filter(node, node_type, args): not args.get('ids') or node_ids_filter(tokens, state, args['ids']) ) - and not ( - args.get('exids') - and node_ids_filter(tokens, state, args['exids']) + and ( + not args.get('exids') + or not node_ids_filter(tokens, state, args['exids']) ) ) @@ -330,14 +329,34 @@ async def get_workflows(self, args): args) # nodes + def get_node_state(self, node, node_type): + """Return state, from node or data-store.""" + if node_type in DEF_TYPES: + return None + # Don't lookup data-store state in getattr, as it might not be there. + # (args are evaluated first) + # If node has no state, it must be in the data-store. + with suppress(Exception): + return ( + getattr(node, 'state', None) + or self.data_store_mgr.data[ + Tokens(node.id).workflow_id + ][node_type][node.id].state + ) + async def get_nodes_all(self, node_type, args): """Return nodes from all workflows, filter by args.""" return sort_elements( [ - n + node for flow in await self.get_workflows_data(args) - for n in flow.get(node_type).values() - if node_filter(n, node_type, args) + for node in flow.get(node_type).values() + if node_filter( + node, + node_type, + args, + self.get_node_state(node, node_type) + ) ], args, ) @@ -366,7 +385,12 @@ async def get_nodes_by_ids(self, node_type, args): for flow in flow_data for node_type in node_types for node in get_data_elements(flow, nat_ids, node_type) - if node_filter(node, node_type, args) + if node_filter( + node, + node_type, + args, + self.get_node_state(node, node_type) + ) ], args, ) From 7fa56dd150868e90c6d03a326defb5f3a3c45b06 Mon Sep 17 00:00:00 2001 From: David Sutherland Date: Thu, 31 Mar 2022 15:44:25 +1300 Subject: [PATCH 3/4] change log entry --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 37a829f7008..aa4d57896b1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -51,6 +51,9 @@ distinguishing between workflow not running and not in run-directory. platforms, any files configured to be installed will be updated on the remote platform. +[#4791](https://github.com/cylc/cylc-flow/pull/4791) - Fix GraphQL node +filtering, effecting non-state updates in the UI. + [#4777](https://github.com/cylc/cylc-flow/pull/4777) - Reinstate the Cylc 7 template variables for xtriggers with deprecation warnings. From 307d1fcb6cd2a8d7e5c6366be4565c1133d1604d Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Wed, 13 Apr 2022 18:09:46 +1200 Subject: [PATCH 4/4] Update CHANGES.md --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index aa4d57896b1..fce7b4179ef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -52,7 +52,7 @@ platforms, any files configured to be installed will be updated on the remote platform. [#4791](https://github.com/cylc/cylc-flow/pull/4791) - Fix GraphQL node -filtering, effecting non-state updates in the UI. +filtering, affecting non-state updates in the UI. [#4777](https://github.com/cylc/cylc-flow/pull/4777) - Reinstate the Cylc 7 template variables for xtriggers with deprecation warnings.