[bug] Fix issue with source position yaml loading (#27160)

## Summary & Motivation This fixes a long-standing bug that would label the source position of the key of a dictionary with the source position of where its value started. ## How I Tested These Changes Note that the changed tests make more sense now ## Changelog NOCHANGELOG
dagster-io · Jan 16, 2025 · dfebf16 · dfebf16
1 parent 42f08b5
commit dfebf16
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 6 deletions.
diff --git a/python_modules/dagster/dagster/_utils/yaml_utils.py b/python_modules/dagster/dagster/_utils/yaml_utils.py
@@ -216,7 +216,10 @@ def construct_object(self, node: Any, deep: Any = False) -> ValueAndSourcePositi
                     Mapping[ValueAndSourcePositionTree, ValueAndSourcePositionTree], value
                 ).items():
                     dict_with_raw_values[k.value] = v.value
-                    child_trees[k.value] = v.source_position_tree
+                    child_trees[k.value] = SourcePositionTree(
+                        position=k.source_position_tree.position,
+                        children=v.source_position_tree.children,
+                    )
                 return ValueAndSourcePositionTree(
                     dict_with_raw_values,
                     SourcePositionTree(position=source_position, children=child_trees),

diff --git a/python_modules/dagster/dagster_tests/utils_tests/test_source_position.py b/python_modules/dagster/dagster_tests/utils_tests/test_source_position.py
@@ -40,13 +40,13 @@ def visit(path: KeyPath, tree: SourcePositionTree):
     visit([], value_and_tree.source_position_tree)
     assert flattened == {
         "": "foo.yaml:2",
-        "foo": "foo.yaml:3",
+        "foo": "foo.yaml:2",
         "foo.bar": "foo.yaml:3",
         "foo.q": "foo.yaml:4",
         "foo.q.0": "foo.yaml:4",
         "foo.q.1": "foo.yaml:4",
         "foo.q.2": "foo.yaml:4",
-        "foo.baz": "foo.yaml:6",
+        "foo.baz": "foo.yaml:5",
         "foo.baz.0": "foo.yaml:6",
         "foo.baz.1": "foo.yaml:7",
         "foo.baz.1.b": "foo.yaml:7",
@@ -95,7 +95,7 @@ def __init__(self):
     populate_source_position_and_key_paths(value, parsed.source_position_tree)
     assert value.child.dicts[3][4]._source_position_and_key_path is not None  # noqa: SLF001
     assert (
-        str(value.child.dicts[3][4]._source_position_and_key_path.source_position) == "<string>:10"  # noqa: SLF001
+        str(value.child.dicts[3][4]._source_position_and_key_path.source_position) == "<string>:9"  # noqa: SLF001
     )
     assert value.child.dicts[3][4]._source_position_and_key_path.key_path == [  # noqa: SLF001
         "child",

diff --git a/...ster-components/dagster_components_tests/integration_tests/validation_tests/test_cases.py b/...ster-components/dagster_components_tests/integration_tests/validation_tests/test_cases.py
@@ -37,7 +37,7 @@ def _validate_error_msg(msg: str) -> None:
     component_path="validation/basic_component_missing_value",
     component_type_filepath=Path(__file__).parent / "basic_components.py",
     should_error=True,
-    validate_error_msg=msg_includes_all_of("component.yaml:4", "params.an_int", "required"),
+    validate_error_msg=msg_includes_all_of("component.yaml:3", "params.an_int", "required"),
     validate_error_msg_additional_cli=msg_includes_all_of(
         "Field `an_int` is required but not provided"
     ),
@@ -77,7 +77,7 @@ def _validate_error_msg(msg: str) -> None:
         component_type_filepath=Path(__file__).parent / "basic_components.py",
         should_error=True,
         validate_error_msg=msg_includes_all_of(
-            "component.yaml:6", "params.nested.foo.an_int", "required"
+            "component.yaml:5", "params.nested.foo.an_int", "required"
         ),
         validate_error_msg_additional_cli=msg_includes_all_of(
             "Field `a_string` is required but not provided"