microsoft · bpkroth · May 28, 2025 · May 28, 2025 · May 28, 2025 · May 28, 2025
@@ -29,6 +29,7 @@
 from collections.abc import Iterator, Mapping
 from contextlib import AbstractContextManager as ContextManager
 from datetime import datetime
+from subprocess import CalledProcessError
 from types import TracebackType
 from typing import Any, Literal
 
@@ -187,15 +188,62 @@ def __init__(  # pylint: disable=too-many-arguments
             tunables: TunableGroups,
             experiment_id: str,
             trial_id: int,
-            root_env_config: str,
+            root_env_config: str | None,
             description: str,
             opt_targets: dict[str, Literal["min", "max"]],
+            git_repo: str | None = None,
+            git_commit: str | None = None,
+            git_rel_root_env_config: str | None = None,
         ):
             self._tunables = tunables.copy()
             self._trial_id = trial_id
             self._experiment_id = experiment_id
-            (self._git_repo, self._git_commit, self._root_env_config) = get_git_info(
-                root_env_config
+            self._abs_root_env_config: str | None
+            if root_env_config is not None:
+                if git_repo or git_commit or git_rel_root_env_config:
+                    # Extra args are only used when restoring an Experiment from the DB.
+                    raise ValueError("Unexpected args: git_repo, git_commit, rel_root_env_config")
+                try:
+                    (
+                        self._git_repo,
+                        self._git_commit,
+                        self._git_rel_root_env_config,
+                        self._abs_root_env_config,
+                    ) = get_git_info(root_env_config)
+                except CalledProcessError as e:
+                    # Note: currently the Experiment schema requires git
+                    # metadata to be set.  We *could* set the git metadata to
+                    # dummy values, but for now we just throw an error.
+                    _LOG.warning(
+                        "Failed to get git info for root_env_config %s: %s",
+                        root_env_config,
+                        e,
+                    )
+                    raise e
+            else:
+                # Restoring from DB.
+                if not (git_repo and git_commit and git_rel_root_env_config):
+                    raise ValueError("Missing args: git_repo, git_commit, rel_root_env_config")
+                self._git_repo = git_repo
+                self._git_commit = git_commit
+                self._git_rel_root_env_config = git_rel_root_env_config
+                # Note: The absolute path to the root config is not stored in the DB,
+                # and resolving it is not always possible, so we omit this
+                # operation by default for now.
+                # See commit 0cb5948865662776e92ceaca3f0a80a34c6a39ef in
+                # <https://github.com/microsoft/MLOS/pull/985> for prior
+                # implementation attempts.
+                self._abs_root_env_config = None
+            assert isinstance(
+                self._git_rel_root_env_config, str
+            ), "Failed to get relative root config path"
+            _LOG.info(
+                "Resolved relative root_config %s from %s at commit %s for Experiment %s to %s",
+                self._git_rel_root_env_config,
+                self._git_repo,
+                self._git_commit,
+                self._experiment_id,
+                self._abs_root_env_config,
             )
             self._description = description
             self._opt_targets = opt_targets
@@ -205,6 +253,8 @@ def __enter__(self) -> Storage.Experiment:
             """
             Enter the context of the experiment.
 
+            Notes
+            -----
             Override the `_setup` method to add custom context initialization.
             """
             _LOG.debug("Starting experiment: %s", self)
@@ -222,6 +272,8 @@ def __exit__(
             """
             End the context of the experiment.
 
+            Notes
+            -----
             Override the `_teardown` method to add custom context teardown logic.
             """
             is_ok = exc_val is None
@@ -247,14 +299,14 @@ def _setup(self) -> None:
             Create a record of the new experiment or find an existing one in the
             storage.
 
-            This method is called by `Storage.Experiment.__enter__()`.
+            This method is called by :py:class:`.Storage.Experiment.__enter__()`.
             """
 
         def _teardown(self, is_ok: bool) -> None:
             """
             Finalize the experiment in the storage.
 
-            This method is called by `Storage.Experiment.__exit__()`.
+            This method is called by :py:class:`.Storage.Experiment.__exit__()`.
 
             Parameters
             ----------
@@ -278,9 +330,35 @@ def description(self) -> str:
             return self._description
 
         @property
-        def root_env_config(self) -> str:
-            """Get the Experiment's root Environment config file path."""
-            return self._root_env_config
+        def rel_root_env_config(self) -> str:
+            """Get the Experiment's root Environment config's relative file path to the
+            git repo root.
+            """
+            return self._git_rel_root_env_config
+
+        @property
+        def abs_root_env_config(self) -> str | None:
+            """
+            Get the Experiment's root Environment config absolute file path.
+
+            This attempts to return the current absolute path to the root config
+            for this process instead of the path relative to the git repo root.
+
+            However, this may not always be possible if the git repo root is not
+            accessible, which can happen if the Experiment was restored from the
+            DB, but the process was started from a different working directory,
+            for instance.
+
+            Notes
+            -----
+            This is mostly useful for other components (e.g.,
+            :py:class:`~mlos_bench.schedulers.base_scheduler.Scheduler`) to use
+            within the same process, and not across invocations.
+            """
+            # TODO: In the future, we can consider fetching the git_repo to a
+            # standard working directory for ``mlos_bench`` and then resolving
+            # the root config path from there based on the relative path.
+            return self._abs_root_env_config
 
         @property
         def tunables(self) -> TunableGroups:

@@ -38,9 +38,12 @@ def __init__(  # pylint: disable=too-many-arguments
         tunables: TunableGroups,
         experiment_id: str,
         trial_id: int,
-        root_env_config: str,
+        root_env_config: str | None,
         description: str,
         opt_targets: dict[str, Literal["min", "max"]],
+        git_repo: str | None = None,
+        git_commit: str | None = None,
+        git_rel_root_env_config: str | None = None,
     ):
         super().__init__(
             tunables=tunables,
@@ -49,6 +52,9 @@ def __init__(  # pylint: disable=too-many-arguments
             root_env_config=root_env_config,
             description=description,
             opt_targets=opt_targets,
+            git_repo=git_repo,
+            git_commit=git_commit,
+            git_rel_root_env_config=git_rel_root_env_config,
         )
         self._engine = engine
         self._schema = schema
@@ -89,7 +95,7 @@ def _setup(self) -> None:
                         description=self._description,
                         git_repo=self._git_repo,
                         git_commit=self._git_commit,
-                        root_env_config=self._root_env_config,
+                        root_env_config=self._git_rel_root_env_config,
                     )
                 )
                 conn.execute(

@@ -156,9 +156,13 @@ def get_experiment_by_id(
                 experiment_id=exp.exp_id,
                 trial_id=-1,  # will be loaded upon __enter__ which calls _setup()
                 description=exp.description,
-                root_env_config=exp.root_env_config,
+                # Use special logic to load the experiment root config info directly.
+                root_env_config=None,
                 tunables=tunables,
                 opt_targets=opt_targets,
+                git_repo=exp.git_repo,
+                git_commit=exp.git_commit,
+                git_rel_root_env_config=exp.root_env_config,
             )
 
     def experiment(  # pylint: disable=too-many-arguments

@@ -30,7 +30,7 @@ def test_exp_data_root_env_config(
     """Tests the root_env_config property of ExperimentData."""
     # pylint: disable=protected-access
     assert exp_data.root_env_config == (
-        exp_storage._root_env_config,
+        exp_storage._git_rel_root_env_config,
         exp_storage._git_repo,
         exp_storage._git_commit,
     )

@@ -241,7 +241,7 @@ def exp_storage(
     with storage.experiment(
         experiment_id="Test-001",
         trial_id=1,
-        root_env_config="environment.jsonc",
+        root_env_config="my-environment.jsonc",
         description="pytest experiment",
         tunables=tunable_groups,
         opt_targets={"score": "min"},
@@ -375,7 +375,7 @@ def _dummy_run_exp(
         trial_runners=trial_runners,
         optimizer=opt,
         storage=storage,
-        root_env_config=exp.root_env_config,
+        root_env_config=exp.abs_root_env_config or "ERROR-UNKNOWN.jsonc",
     )
 
     # Add some trial data to that experiment by "running" it.

@@ -17,9 +17,8 @@
 # See Also: schema.py for an example of programmatic alembic config access.
 CURRENT_ALEMBIC_HEAD = "b61aa446e724"
 
-# Try to test multiple DBMS engines.
-
 
+# Try to test multiple DBMS engines.
 @pytest.mark.parametrize(
     "some_sql_storage_fixture",
     [

@@ -72,7 +72,7 @@ def test_storage_pickle_restore_experiment_and_trial(
     assert restored_experiment is not experiment
     assert restored_experiment.experiment_id == experiment.experiment_id
     assert restored_experiment.description == experiment.description
-    assert restored_experiment.root_env_config == experiment.root_env_config
+    assert restored_experiment.rel_root_env_config == experiment.rel_root_env_config
     assert restored_experiment.tunables == experiment.tunables
     assert restored_experiment.opt_targets == experiment.opt_targets
     with restored_experiment:

@@ -3,14 +3,80 @@
 # Licensed under the MIT License.
 #
 """Unit tests for get_git_info utility function."""
+import os
 import re
+import tempfile
+from pathlib import Path
+from subprocess import CalledProcessError
 
-from mlos_bench.util import get_git_info
+import pytest
+
+from mlos_bench.util import get_git_info, get_git_root, path_join
 
 
 def test_get_git_info() -> None:
-    """Check that we can retrieve git info about the current repository correctly."""
-    (git_repo, git_commit, rel_path) = get_git_info(__file__)
+    """Check that we can retrieve git info about the current repository correctly from a
+    file.
+    """
+    (git_repo, git_commit, rel_path, abs_path) = get_git_info(__file__)
     assert "mlos" in git_repo.lower()
     assert re.match(r"[0-9a-f]{40}", git_commit) is not None
     assert rel_path == "mlos_bench/mlos_bench/tests/util_git_test.py"
+    assert abs_path == path_join(__file__, abs_path=True)
+
+
+def test_get_git_info_dir() -> None:
+    """Check that we can retrieve git info about the current repository correctly from a
+    directory.
+    """
+    dirname = os.path.dirname(__file__)
+    (git_repo, git_commit, rel_path, abs_path) = get_git_info(dirname)
+    assert "mlos" in git_repo.lower()
+    assert re.match(r"[0-9a-f]{40}", git_commit) is not None
+    assert rel_path == "mlos_bench/mlos_bench/tests"
+    assert abs_path == path_join(dirname, abs_path=True)
+
+
+def test_non_git_dir() -> None:
+    """Check that we can handle a non-git directory."""
+    with tempfile.TemporaryDirectory() as non_git_dir:
+        with pytest.raises(CalledProcessError):
+            # This should raise an error because the directory is not a git repository.
+            get_git_root(non_git_dir)
+
+
+def test_non_upstream_git() -> None:
+    """Check that we can handle a git directory without an upstream."""
+    with tempfile.TemporaryDirectory() as non_upstream_git_dir:
+        non_upstream_git_dir = path_join(non_upstream_git_dir, abs_path=True)
+        # Initialize a new git repository.
+        os.system(f"git init {non_upstream_git_dir} -b main")
+        os.system(f"git -C {non_upstream_git_dir} config --local user.email 'pytest@example.com'")
+        os.system(f"git -C {non_upstream_git_dir} config --local user.name 'PyTest User'")
+        Path(non_upstream_git_dir).joinpath("README.md").touch()
+        os.system(f"git -C {non_upstream_git_dir} add README.md")
+        os.system(f"git -C {non_upstream_git_dir} commit -m 'Initial commit'")
+        # This should raise an error because the repository has no upstream.
+        (git_repo, _git_commit, rel_path, abs_path) = get_git_info(non_upstream_git_dir)
+        assert git_repo == f"file://{non_upstream_git_dir}"
+        assert abs_path == non_upstream_git_dir
+        assert rel_path == "."
+
+
+@pytest.mark.skipif(
+    os.environ.get("GITHUB_ACTIONS") != "true",
+    reason="Not running in GitHub Actions CI.",
+)
+def test_github_actions_git_info() -> None:
+    """Test that get_git_info matches GitHub Actions environment variables if running in
+    CI.
+    """
+    repo_env = os.environ.get("GITHUB_REPOSITORY")  # "owner/repo" format
+    sha_env = os.environ.get("GITHUB_SHA")
+    assert repo_env, "GITHUB_REPOSITORY not set in environment."
+    assert sha_env, "GITHUB_SHA not set in environment."
+    git_repo, git_commit, _rel_path, _abs_path = get_git_info(__file__)
+    assert git_repo.endswith(repo_env), f"git_repo '{git_repo}' does not end with '{repo_env}'"
+    assert (
+        git_commit == sha_env
+    ), f"git_commit '{git_commit}' does not match GITHUB_SHA '{sha_env}'"