Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions integrations/harbor/_load.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ def load(path: str | Path, *, images: dict[str, str] | None = None) -> Taskset:
"""Load a Harbor task dir (or dataset dir) into a :class:`Taskset`.

One row per task dir (``id`` = the dir name); rows share one env name per
distinct ``environment/`` build context (content-hashed), derived from
the dataset name. Each row carries the task's declared launch
distinct environment (see :func:`grouped` for what distinguishes them and
how the name is derived). Each row carries the task's declared launch
requirements (:func:`runtime_config`: cpu/memory/gpu and time budgets),
plus the adapted image ref once
:func:`~harbor.adapt` has produced it, so it runs on any
Expand Down Expand Up @@ -387,11 +387,15 @@ def final_stage(dockerfile_text: str) -> FinalStage:


def grouped(root: str | Path) -> list[tuple[str, list[Path]]]:
"""Task dirs grouped by identical ``environment/`` content, largest first.

One env name per group (the dataset slug, ``-gN``-suffixed when there are
several): the join key between :func:`load`'s rows and
:func:`~harbor.adapt`'s images.
"""Task dirs grouped by the env they need, under content-derived names.

One env name per group — ``<dataset>-<digest>`` over everything that
decides what the group's image is — and that name is the join key
between :func:`load`'s rows and :func:`~harbor.adapt`'s images. Deriving
it from content rather than position is what makes the join safe: a name
denotes one image forever, so editing, adding or removing a task can
never leave a row pointing at an environment that has since come to mean
something else.
"""
resolved = Path(root).resolve()
dataset_name = resolved.parent.name if is_harbor_task(resolved) else resolved.name
Expand All @@ -411,11 +415,17 @@ def grouped(root: str | Path) -> list[tuple[str, list[Path]]]:
# for the whole group.
policy = json.dumps(workspace_policy(task_dir), sort_keys=True)
groups.setdefault((env_hash, policy), []).append(task_dir)
ordered = sorted(groups.values(), key=lambda group: -len(group))
base_name = slugify(dataset_name)
if len(ordered) == 1:
return [(base_name, ordered[0])]
return [(f"{base_name}-g{idx}", group) for idx, group in enumerate(ordered, start=1)]
return sorted(
(f"{base_name}-{_group_digest(env_hash, policy)}", group)
for (env_hash, policy), group in groups.items()
)


def _group_digest(env_hash: str, policy: str) -> str:
"""Short stable digest of one group's key — its build context and the
workspace policy its image bakes in, which together are the image."""
return hashlib.sha256(f"{env_hash}\0{policy}".encode()).hexdigest()[:12]


# ─── task-dir primitives ────────────────────────────────────────────────
Expand Down
39 changes: 36 additions & 3 deletions integrations/harbor/tests/test_harbor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,60 @@ def test_load_single_task_dir_maps_rows(single_task: Path) -> None:
row = taskset["cancel-async-tasks"]
assert row.id == "cancel-async-tasks"
assert row.args == {}
assert row.env == taskset.name
assert row.env.startswith(f"{taskset.name}-")


def test_load_dataset_shares_one_env_per_build_context(dataset_same_env: Path) -> None:
taskset = load(dataset_same_env)

assert len(taskset) == 3
# Identical Dockerfiles -> all rows reference one env name.
assert taskset.environment_names() == {"terminal-bench-sample"}
(env_name,) = taskset.environment_names()
assert env_name.startswith("terminal-bench-sample-")


def test_load_dataset_groups_by_distinct_build_contexts(dataset_multi_env: Path) -> None:
taskset = load(dataset_multi_env)

assert len(taskset) == 4
assert taskset.environment_names() == {"mixed-bench-g1", "mixed-bench-g2"}
names = taskset.environment_names()
assert len(names) == 2
assert all(name.startswith("mixed-bench-") for name in names)
assert taskset["build-pmars"].env == taskset["cancel-async-tasks"].env
assert taskset["caffe-cifar-10"].env == taskset["sam-cell-seg"].env
assert taskset["build-pmars"].env != taskset["caffe-cifar-10"].env


def test_env_name_survives_other_tasks_joining_the_dataset(tmp_path: Path) -> None:
"""A row's env name follows its own environment, not the dataset's shape.

Names used to be positional (``-g1``, ``-g2``, assigned largest group
first), so growing one group renumbered the others and rows kept naming
an environment that had come to mean a different image.
"""
dataset = tmp_path / "bench"
dataset.mkdir()
solo_image, pair_image = "FROM alpine:3\n", "FROM debian:12\n"
make_harbor_task(dataset, "solo", dockerfile=solo_image)
for name in ("pair-a", "pair-b"):
make_harbor_task(dataset, name, dockerfile=pair_image)

before = load(dataset)
solo_env = before["solo"].env
pair_env = before["pair-a"].env
assert solo_env != pair_env

# The smaller group overtakes the larger one; under positional naming the
# two names swapped, silently re-pointing every row in both.
for name in ("solo-b", "solo-c"):
make_harbor_task(dataset, name, dockerfile=solo_image)

after = load(dataset)
assert after["solo"].env == solo_env
assert after["pair-a"].env == pair_env
assert after["solo-b"].env == solo_env


def test_load_rejects_dirs_without_harbor_tasks(tmp_path: Path) -> None:
empty = tmp_path / "empty"
empty.mkdir()
Expand Down
Loading