feat: perform deep merges of extra runtime options (#36)

* feat: perform deep merges of extra runtime options

* refactor: remove validation of overridden values

.github/ISSUE_TEMPLATE/new-feature.md

@@ -1,7 +1,7 @@
 ---
 name: Feature Request
 about: Do you want to propose a new feature?
-labels: feature
+labels: enhancement
 ---
 
 #### What are you trying to do:

dagger/runtime/argo/extra_spec_options.py

@@ -44,13 +44,28 @@ def with_extra_spec_options(
     ValueError
         If we attempt to override keys that are already present in the original mapping.
     """
-    if not extra_options:
-        return original
+    spec = {**original}
 
-    key_intersection = set(extra_options).intersection(original)
-    if key_intersection:
-        raise ValueError(
-            f"In {context}, you are trying to override the value of {sorted(list(key_intersection))}. The Argo runtime uses these attributes to guarantee the behavior of the supplied DAG is correct. Therefore, we cannot let you override them."
-        )
+    for key_to_override, overridden_value in extra_options.items():
+        this_context = ".".join([context, key_to_override])
 
-    return {**original, **extra_options}
+        if key_to_override not in spec:
+            spec[key_to_override] = overridden_value
+        elif isinstance(spec[key_to_override], list) and isinstance(
+            overridden_value, list
+        ):
+            spec[key_to_override] += overridden_value
+        elif isinstance(spec[key_to_override], dict) and isinstance(
+            overridden_value, dict
+        ):
+            spec[key_to_override] = with_extra_spec_options(
+                original=spec[key_to_override],
+                extra_options=overridden_value,
+                context=this_context,
+            )
+        else:
+            raise ValueError(
+                f"You are trying to override the value of '{this_context}'. The Argo runtime already sets a value for this key, and it uses it to guarantee the correctness of the behavior. Therefore, we cannot let you override them."
+            )
+
+    return spec

dagger/runtime/argo/workflow_spec.py

@@ -232,7 +232,7 @@ def _dag_template(
     template["dag"] = with_extra_spec_options(
         original=template["dag"],
         extra_options=dag.runtime_options.get("argo_dag_template_overrides", {}),
-        context=".".join(address) if address else "this DAG",
+        context=".".join(address) if address else "DAG",
     )
 
     return template

tests/runtime/argo/test_extra_spec_options.py

@@ -8,19 +8,44 @@ def test__with_extra_options__with_empty_overrides():
     assert with_extra_spec_options(original, {}, "my context") == original
 
 
+def test__with_extra_options__with_deep_overrides():
+    original = {
+        "a": {
+            "a": 1,
+            "b": [1],
+        },
+    }
+    assert with_extra_spec_options(
+        original,
+        {
+            "a": {
+                "b": [2],
+                "c": 2,
+            },
+        },
+        "my context",
+    ) == {
+        "a": {
+            "a": 1,
+            "b": [1, 2],
+            "c": 2,
+        }
+    }
+
+
 def test__with_extra_options__overriding_existing_attributes():
-    original = {"z": 2, "a": 1, "b": 3}
+    original = {"z": 2, "a": {"a": 1}, "b": 3}
 
     with pytest.raises(ValueError) as e:
-        with_extra_spec_options(original, {"z": 4, "a": 2}, "my context")
+        with_extra_spec_options(original, {"y": 3, "a": {"a": 2}}, "my context")
 
     assert (
         str(e.value)
-        == "In my context, you are trying to override the value of ['a', 'z']. The Argo runtime uses these attributes to guarantee the behavior of the supplied DAG is correct. Therefore, we cannot let you override them."
+        == "You are trying to override the value of 'my context.a.a'. The Argo runtime already sets a value for this key, and it uses it to guarantee the correctness of the behavior. Therefore, we cannot let you override them."
     )
 
 
-def test__with_extra_options__setting_extra_options():
+def test__with_extra_options__setting_extra_options_that_didnt_exist():
     original = {"a": 1}
     assert with_extra_spec_options(original, {"b": 2, "c": 3}, "my context") == {
         "a": 1,

tests/runtime/argo/test_workflow_spec.py

@@ -237,7 +237,9 @@ def test__workflow_spec__with_template_overrides_that_affect_essential_attribute
                 lambda: 1,
                 runtime_options={
                     "argo_template_overrides": {
-                        "container": {},
+                        "container": {
+                            "image": "a-different-image",
+                        },
                         "name": "x",
                     }
                 },
@@ -250,7 +252,7 @@ def test__workflow_spec__with_template_overrides_that_affect_essential_attribute
 
     assert (
         str(e.value)
-        == "In n, you are trying to override the value of ['container', 'name']. The Argo runtime uses these attributes to guarantee the behavior of the supplied DAG is correct. Therefore, we cannot let you override them."
+        == "You are trying to override the value of 'n.container.image'. The Argo runtime already sets a value for this key, and it uses it to guarantee the correctness of the behavior. Therefore, we cannot let you override them."
     )
 
 
@@ -273,7 +275,7 @@ def test__workflow_spec__with_container_overrides_that_affect_essential_attribut
 
     assert (
         str(e.value)
-        == "In n, you are trying to override the value of ['image']. The Argo runtime uses these attributes to guarantee the behavior of the supplied DAG is correct. Therefore, we cannot let you override them."
+        == "You are trying to override the value of 'n.image'. The Argo runtime already sets a value for this key, and it uses it to guarantee the correctness of the behavior. Therefore, we cannot let you override them."
     )
 
 
@@ -357,7 +359,7 @@ def test__workflow_spec__with_dag_template_overrides_that_affect_essential_attri
         {"n": Task(lambda: 1)},
         runtime_options={
             "argo_dag_template_overrides": {
-                "tasks": [],
+                "tasks": None,
             },
         },
     )
@@ -367,7 +369,7 @@ def test__workflow_spec__with_dag_template_overrides_that_affect_essential_attri
 
     assert (
         str(e.value)
-        == "In this DAG, you are trying to override the value of ['tasks']. The Argo runtime uses these attributes to guarantee the behavior of the supplied DAG is correct. Therefore, we cannot let you override them."
+        == "You are trying to override the value of 'DAG.tasks'. The Argo runtime already sets a value for this key, and it uses it to guarantee the correctness of the behavior. Therefore, we cannot let you override them."
     )