From c8d59cdca56025f099027a9a04e9dd65cdb88d60 Mon Sep 17 00:00:00 2001 From: Johann Miller Date: Mon, 16 Oct 2023 11:52:35 -0400 Subject: [PATCH 1/2] Fix error on op selection --- python_modules/dagster-graphql/dagster_graphql/test/utils.py | 2 ++ python_modules/dagster/dagster/_core/instance/__init__.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/python_modules/dagster-graphql/dagster_graphql/test/utils.py b/python_modules/dagster-graphql/dagster_graphql/test/utils.py index 9b74d12500b69..874ae986fe271 100644 --- a/python_modules/dagster-graphql/dagster_graphql/test/utils.py +++ b/python_modules/dagster-graphql/dagster_graphql/test/utils.py @@ -201,6 +201,8 @@ def infer_pipeline_selector( { "pipelineName": pipeline_name, "solidSelection": op_selection, + "assetSelection": [], + "assetCheckSelection": [], } ) return selector diff --git a/python_modules/dagster/dagster/_core/instance/__init__.py b/python_modules/dagster/dagster/_core/instance/__init__.py index 8fec0e07392e1..73162ca0ac344 100644 --- a/python_modules/dagster/dagster/_core/instance/__init__.py +++ b/python_modules/dagster/dagster/_core/instance/__init__.py @@ -1461,7 +1461,9 @@ def create_run( check.opt_set_param(asset_selection, "asset_selection", of_type=AssetKey) check.opt_set_param(asset_check_selection, "asset_check_selection", of_type=AssetCheckKey) - if asset_selection is not None or asset_check_selection is not None: + # asset_selection gets coerced from [] to None, but asset_check_selection doesn't. We + # allow asset_check_selection to be [] when op_selection is set. + if asset_selection is not None or asset_check_selection: check.invariant( op_selection is None, "Cannot pass op_selection with either of asset_selection or asset_check_selection", From f98f0f795df0f794f06139cdba9a117cbc76fbfc Mon Sep 17 00:00:00 2001 From: Johann Miller Date: Mon, 16 Oct 2023 14:48:40 -0400 Subject: [PATCH 2/2] cover [] and none --- .../dagster_graphql/test/utils.py | 6 +++-- .../graphql/test_execute_pipeline.py | 27 +++++++++++++++++++ .../dagster/_core/instance/__init__.py | 6 +++-- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/python_modules/dagster-graphql/dagster_graphql/test/utils.py b/python_modules/dagster-graphql/dagster_graphql/test/utils.py index 874ae986fe271..6b7bd62153359 100644 --- a/python_modules/dagster-graphql/dagster_graphql/test/utils.py +++ b/python_modules/dagster-graphql/dagster_graphql/test/utils.py @@ -195,14 +195,16 @@ def infer_pipeline_selector( graphql_context: WorkspaceRequestContext, pipeline_name: str, op_selection: Optional[Sequence[str]] = None, + asset_selection: Optional[Sequence[GqlAssetKey]] = None, + asset_check_selection: Optional[Sequence[GqlAssetCheckHandle]] = None, ) -> Selector: selector = infer_repository_selector(graphql_context) selector.update( { "pipelineName": pipeline_name, "solidSelection": op_selection, - "assetSelection": [], - "assetCheckSelection": [], + "assetSelection": asset_selection, + "assetCheckSelection": asset_check_selection, } ) return selector diff --git a/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py b/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py index 57a519c1394d6..cee22cb9b8674 100644 --- a/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py +++ b/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py @@ -687,6 +687,33 @@ def test_nested_graph_op_selection_and_config(self, graphql_context: WorkspaceRe assert step_did_not_run(logs, "plus_one") assert step_did_not_run(logs, "subgraph.op_2") + def test_nested_graph_op_selection_and_config_with_non_null_asset_and_check_selection( + self, graphql_context: WorkspaceRequestContext + ): + selector = infer_pipeline_selector( + graphql_context, + "nested_job", + ["subgraph.adder", "subgraph.op_1"], + asset_selection=[], + asset_check_selection=[], + ) + run_config = {"ops": {"subgraph": {"ops": {"adder": {"inputs": {"num2": 20}}}}}} + result = sync_execute_get_run_log_data( + context=graphql_context, + variables={ + "executionParams": { + "selector": selector, + "runConfigData": run_config, + } + }, + ) + logs = result["messages"] + assert isinstance(logs, list) + assert step_did_succeed(logs, "subgraph.adder") + assert step_did_succeed(logs, "subgraph.op_1") + assert step_did_not_run(logs, "plus_one") + assert step_did_not_run(logs, "subgraph.op_2") + def test_memoization_job(self, graphql_context: WorkspaceRequestContext): selector = infer_pipeline_selector(graphql_context, "memoization_job") run_config = {} diff --git a/python_modules/dagster/dagster/_core/instance/__init__.py b/python_modules/dagster/dagster/_core/instance/__init__.py index 73162ca0ac344..4e0fd399acf7d 100644 --- a/python_modules/dagster/dagster/_core/instance/__init__.py +++ b/python_modules/dagster/dagster/_core/instance/__init__.py @@ -1461,8 +1461,10 @@ def create_run( check.opt_set_param(asset_selection, "asset_selection", of_type=AssetKey) check.opt_set_param(asset_check_selection, "asset_check_selection", of_type=AssetCheckKey) - # asset_selection gets coerced from [] to None, but asset_check_selection doesn't. We - # allow asset_check_selection to be [] when op_selection is set. + # asset_selection will always be None on an op job, but asset_check_selection may be + # None or []. This is because [] and None are different for asset checks: None means + # include all asset checks on selected assets, while [] means include no asset checks. + # In an op job (which has no asset checks), these two are equivalent. if asset_selection is not None or asset_check_selection: check.invariant( op_selection is None,