[ci][deps] raydepsts: Improved error messages

ci/raydepsets/cli.py

@@ -177,7 +177,9 @@ def _build(self):
                 for depset_name in depset.depsets:
                     self.build_graph.add_edge(depset_name, depset.name)
             else:
-                raise ValueError(f"Invalid operation: {depset.operation}")
+                raise ValueError(
+                    f"Invalid operation: {depset.operation} for depset {depset.name} in config {depset.config_name}"
+                )
             if depset.pre_hooks:
                 for ind, hook in enumerate(depset.pre_hooks):
                     hook_name = f"{depset.name}_pre_hook_{ind+1}"
@@ -232,24 +234,27 @@ def exec_uv_cmd(
         self, cmd: str, args: List[str], stdin: Optional[bytes] = None
     ) -> str:
         cmd = [self._uv_binary, "pip", cmd, *args]
-        click.echo(f"Executing command: {cmd}")
-        status = subprocess.run(cmd, cwd=self.workspace.dir, input=stdin)
+        click.echo(f"Executing command: {' '.join(cmd)}")
+        status = subprocess.run(
+            cmd, cwd=self.workspace.dir, input=stdin, capture_output=True
+        )
         if status.returncode != 0:
-            raise RuntimeError(f"Failed to execute command: {cmd}")
-        return status.stdout
+            raise RuntimeError(
+                f"Failed to execute command: {' '.join(cmd)} with error: {status.stderr.decode('utf-8')}"
+            )
+        return status.stdout.decode("utf-8")
 
     def execute_pre_hook(self, pre_hook: str):
         status = subprocess.run(
             shlex.split(pre_hook),
             cwd=self.workspace.dir,
             capture_output=True,
-            text=True,
         )
         if status.returncode != 0:
             raise RuntimeError(
-                f"Failed to execute pre_hook {pre_hook} with error: {status.stderr}",
+                f"Failed to execute pre_hook {pre_hook} with error: {status.stderr.decode('utf-8')}",
             )
-        click.echo(f"{status.stdout}")
+        click.echo(f"{status.stdout.decode('utf-8')}")
         click.echo(f"Executed pre_hook {pre_hook} successfully")
 
     def execute_depset(self, depset: Depset):
@@ -379,7 +384,7 @@ def check_subset_exists(self, source_depset: Depset, requirements: List[str]):
         for req in requirements:
             if req not in source_depset.requirements:
                 raise RuntimeError(
-                    f"Requirement {req} is not a subset of {source_depset.name}"
+                    f"Requirement {req} is not a subset of {source_depset.name} in config {source_depset.config_name}"
                 )
 
     def get_expanded_depset_requirements(

ci/raydepsets/tests/test_cli.py

@@ -27,6 +27,7 @@
     replace_in_file,
     save_file_as,
     save_packages_to_file,
+    write_to_config_file,
 )
 from ci.raydepsets.workspace import (
     Depset,
@@ -67,20 +68,6 @@ def _invoke_build(tmpdir: str, config_path: str, name: Optional[str] = None):
     )
 
 
-def _write_to_config_file(tmpdir: str, depset: Depset, config_name: str):
-    with open(Path(tmpdir) / config_name, "w") as f:
-        f.write(
-            f"""
-depsets:
-    - name: {depset.name}
-      operation: {depset.operation}
-      {f"constraints: {depset.constraints}" if depset.constraints else ""}
-      {f"requirements: {depset.requirements}" if depset.requirements else ""}
-      output: {depset.output}
-                """
-        )
-
-
 class TestCli(unittest.TestCase):
     def test_cli_load_fail_no_config(self):
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -99,13 +86,6 @@ def test_dependency_set_manager_init(self):
             assert len(manager.config.depsets) > 0
             assert len(manager.build_graph.nodes) > 0
 
-    def test_get_depset(self):
-        with tempfile.TemporaryDirectory() as tmpdir:
-            copy_data_to_tmpdir(tmpdir)
-            manager = _create_test_manager(tmpdir)
-            with self.assertRaises(KeyError):
-                _get_depset(manager.config.depsets, "fake_depset")
-
     def test_uv_binary_exists(self):
         assert _uv_binary() is not None
 
@@ -248,14 +228,18 @@ def test_subset_does_not_exist(self):
                 output="requirements_compiled_general.txt",
             )
 
-            with self.assertRaises(RuntimeError):
+            with self.assertRaises(RuntimeError) as e:
                 manager.subset(
                     source_depset="general_depset__py311_cpu",
                     requirements=["requirements_compiled_test.txt"],
                     append_flags=["--no-annotate", "--no-header"],
                     name="subset_general_depset__py311_cpu",
                     output="requirements_compiled_subset_general.txt",
                 )
+            assert (
+                "Requirement requirements_compiled_test.txt is not a subset of general_depset__py311_cpu in config test.depsets.yaml"
+                in str(e.exception)
+            )
 
     def test_check_if_subset_exists(self):
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -271,23 +255,28 @@ def test_check_if_subset_exists(self):
                 override_flags=[],
                 config_name="test.depsets.yaml",
             )
-            with self.assertRaises(RuntimeError):
+            with self.assertRaises(RuntimeError) as e:
                 manager.check_subset_exists(
                     source_depset=source_depset,
                     requirements=["requirements_3.txt"],
                 )
+            assert (
+                "Requirement requirements_3.txt is not a subset of general_depset__py311_cpu in config test.depsets.yaml"
+                in str(e.exception)
+            )
 
     def test_compile_bad_requirements(self):
         with tempfile.TemporaryDirectory() as tmpdir:
             copy_data_to_tmpdir(tmpdir)
             manager = _create_test_manager(tmpdir)
-            with self.assertRaises(RuntimeError):
+            with self.assertRaises(RuntimeError) as e:
                 manager.compile(
                     constraints=[],
                     requirements=["requirements_test_bad.txt"],
                     name="general_depset",
                     output="requirements_compiled_general.txt",
                 )
+            assert "File not found: `requirements_test_bad.txt" in str(e.exception)
 
     def test_get_path(self):
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -436,9 +425,13 @@ def test_build_graph_bad_operation(self):
                 output="requirements_compiled_invalid_op.txt",
                 config_name="test.depsets.yaml",
             )
-            _write_to_config_file(tmpdir, depset, "test.depsets.yaml")
-            with self.assertRaises(ValueError):
+            write_to_config_file(tmpdir, depset, "test.depsets.yaml")
+            with self.assertRaises(ValueError) as e:
                 _create_test_manager(tmpdir)
+            assert (
+                "Invalid operation: invalid_op for depset invalid_op_depset in config test.depsets.yaml"
+                in str(e.exception)
+            )
 
     def test_execute(self):
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -459,8 +452,9 @@ def test_execute_single_depset_that_does_not_exist(self):
         with tempfile.TemporaryDirectory() as tmpdir:
             copy_data_to_tmpdir(tmpdir)
             manager = _create_test_manager(tmpdir)
-            with self.assertRaises(KeyError):
+            with self.assertRaises(KeyError) as e:
                 manager.execute(single_depset_name="fake_depset")
+            assert "Dependency set fake_depset not found" in str(e.exception)
 
     def test_expand(self):
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -565,16 +559,6 @@ def test_get_depset_without_build_arg_set(self):
             depset = _get_depset(manager.config.depsets, "ray_base_test_depset")
             assert depset.name == "ray_base_test_depset"
 
-    def test_get_depset_with_build_arg_set_and_no_build_arg_set_provided(self):
-        with tempfile.TemporaryDirectory() as tmpdir:
-            copy_data_to_tmpdir(tmpdir)
-            manager = DependencySetManager(
-                config_path="test.depsets.yaml",
-                workspace_dir=tmpdir,
-            )
-            with self.assertRaises(KeyError):
-                _get_depset(manager.config.depsets, "build_args_test_depset_py311")
-
     def test_execute_single_pre_hook(self):
         with tempfile.TemporaryDirectory() as tmpdir:
             copy_data_to_tmpdir(tmpdir)
@@ -608,7 +592,7 @@ def test_copy_lock_files_to_temp_dir(self):
                 output="requirements_compiled_test.txt",
                 config_name="test.depsets.yaml",
             )
-            _write_to_config_file(tmpdir, depset, "test.depsets.yaml")
+            write_to_config_file(tmpdir, depset, "test.depsets.yaml")
             save_file_as(
                 Path(tmpdir) / "requirements_compiled_test.txt",
                 Path(tmpdir) / "requirements_compiled.txt",
@@ -637,7 +621,7 @@ def test_diff_lock_files_out_of_date(self):
                 output="requirements_compiled_test.txt",
                 config_name="test.depsets.yaml",
             )
-            _write_to_config_file(tmpdir, depset, "test.depsets.yaml")
+            write_to_config_file(tmpdir, depset, "test.depsets.yaml")
             manager = _create_test_manager(tmpdir, check=True)
             manager.compile(
                 constraints=["requirement_constraints_test.txt"],
@@ -672,7 +656,7 @@ def test_diff_lock_files_up_to_date(self):
                 output="requirements_compiled_test.txt",
                 config_name="test.depsets.yaml",
             )
-            _write_to_config_file(tmpdir, depset, "test.depsets.yaml")
+            write_to_config_file(tmpdir, depset, "test.depsets.yaml")
             manager = _create_test_manager(tmpdir, check=True)
             manager.compile(
                 constraints=["requirement_constraints_test.txt"],

ci/raydepsets/tests/test_workspace.py

@@ -1,11 +1,21 @@
 import sys
 import tempfile
+import unittest
 from pathlib import Path
 
 import pytest
 
-from ci.raydepsets.tests.utils import copy_data_to_tmpdir, get_depset_by_name
-from ci.raydepsets.workspace import BuildArgSet, Workspace, _substitute_build_args
+from ci.raydepsets.tests.utils import (
+    copy_data_to_tmpdir,
+    get_depset_by_name,
+    write_to_config_file,
+)
+from ci.raydepsets.workspace import (
+    BuildArgSet,
+    Depset,
+    Workspace,
+    _substitute_build_args,
+)
 
 
 def test_workspace_init():
@@ -134,5 +144,31 @@ def test_get_configs_dir():
         assert f"{tmpdir}/test2.depsets.yaml" in configs_dir
 
 
+def test_invalid_build_arg_set_in_config():
+    with tempfile.TemporaryDirectory() as tmpdir:
+        copy_data_to_tmpdir(tmpdir)
+        depset = Depset(
+            name="invalid_build_arg_set",
+            operation="compile",
+            requirements=["requirements_test.txt"],
+            output="requirements_compiled_invalid_build_arg_set.txt",
+            config_name="test.depsets.yaml",
+        )
+        write_to_config_file(
+            tmpdir,
+            depset,
+            "test.depsets.yaml",
+            build_arg_sets=["invalid_build_arg_set"],
+        )
+        workspace = Workspace(dir=tmpdir)
+        with unittest.TestCase().assertRaises(KeyError) as e:
+            workspace.load_config(config_path=Path(tmpdir) / "test.depsets.yaml")
+        print(str(e.exception))
+        assert (
+            "Build arg set invalid_build_arg_set not found in config test.depsets.yaml"
+            in str(e.exception)
+        )
+
+
 if __name__ == "__main__":
     sys.exit(pytest.main(["-v", __file__]))

ci/raydepsets/tests/utils.py

@@ -1,10 +1,13 @@
 """Shared test utilities for raydepsets tests."""
 
 import shutil
-from typing import Optional
+from pathlib import Path
+from typing import List, Optional
 
 import runfiles
 
+from ci.raydepsets.workspace import Depset
+
 _REPO_NAME = "io_ray"
 _runfiles = runfiles.Create()
 
@@ -51,3 +54,27 @@ def get_depset_by_name(depsets, name):
     for depset in depsets:
         if depset.name == name:
             return depset
+
+
+def write_to_config_file(
+    tmpdir: str, depset: Depset, config_name: str, build_arg_sets: List[str] = None
+):
+    with open(Path(tmpdir) / config_name, "w") as f:
+        f.write(
+            f"""
+depsets:
+    - name: {depset.name}
+      operation: {depset.operation}
+      {f"constraints: {depset.constraints}" if depset.constraints else ""}
+      {f"requirements: {depset.requirements}" if depset.requirements else ""}
+      output: {depset.output}
+      {f"pre_hooks: {depset.pre_hooks}" if depset.pre_hooks else ""}
+      {f"depsets: {depset.depsets}" if depset.depsets else ""}
+      {f"source_depset: {depset.source_depset}" if depset.source_depset else ""}
+      {f"config_name: {depset.config_name}" if depset.config_name else ""}
+      {f"append_flags: {depset.append_flags}" if depset.append_flags else ""}
+      {f"override_flags: {depset.override_flags}" if depset.override_flags else ""}
+      {f"packages: {depset.packages}" if depset.packages else ""}
+      {f"build_arg_sets: {build_arg_sets}" if build_arg_sets else ""}
+                """
+        )

ci/raydepsets/workspace.py

@@ -72,9 +72,12 @@ def from_dict(cls, data: dict, config_name: str) -> "Config":
             if build_arg_set_keys:
                 # Expand the depset for each build arg set
                 for build_arg_set_key in build_arg_set_keys:
-                    build_arg_set = build_arg_sets[build_arg_set_key]
-                    if build_arg_set is None:
-                        raise KeyError(f"Build arg set {build_arg_set_key} not found")
+                    try:
+                        build_arg_set = build_arg_sets[build_arg_set_key]
+                    except KeyError:
+                        raise KeyError(
+                            f"Build arg set {build_arg_set_key} not found in config {config_name}"
+                        )
                     depset_yaml = _substitute_build_args(depset, build_arg_set)
                     depsets.append(_dict_to_depset(depset_yaml, config_name))
             else: