Generic sdist-build hooks in conda; port prebuilt sdist/git support

[npow]

Summary

Adds generic conda sdist-build-deferral hooks to metaflow-netflixext and ships the complete metaflow-prebuilt orchestration package for `--environment=prebuilt` — a workflow that pre-bakes resolved conda environments into Docker images so remote tasks bootstrap in seconds rather than minutes.

metaflow-netflixext (conda core changes)

Two primitives, each default-off — non-prebuilt resolve/create is byte-for-byte unchanged:

• `pip_resolver.defer_pypi_sdist_build` — records sdists as `{name, version, url}` in `deferred_sdists` instead of building wheels at resolution time.
• `conda._create(..., only_binary=True)` — installs only pre-built wheels (analogous to `pip --only-binary=:all:`); threaded through `create_for_step` / `create_for_name`.
• `conda_environment._make_envs_resolver()` — zero-arg factory hook so a subclass can opt into deferral without duplicating `init_environment`.

Conda core contains zero references to `prebuilt`, `deferred_builds.json`, or `schema_version`.

metaflow-prebuilt (owns the protocol)

• `prebuilt_conda_environment`: opts into deferral via `_make_envs_resolver`; materializes wheels for non-web (git/local) packages; writes `deferred_builds.json` (schema v2) and COPYs it + `deferred_wheels/` into the Dockerfile.
• Two-pass installer (Pass A / Pass B):
  • Pass A: `create(only_binary=True)` — conda + pre-built wheels, fully offline.
  • Pass B: `uv pip install --no-build-isolation --no-deps` — builds deferred sdists in-container, including `[build-system].requires` packages (e.g. deepspeed).
• Image identity: keyed by environment + base-image/arch variant; schema v29.
• Cross-arch builds: `--platform` flag passed through to the build service.
• Pluggable build services (docker/buildx/kaniko/codebuild) and image registries (ecr/gcr/local) registered via entry points.

Also restores deepspeed (sdist) and transnetv2 (git) support that was developed inline in the internal repo but never ported to the OSS packages.

Validation

• 77 prebuilt unit tests pass.
• Validated end-to-end on real Netflix Titus (linked runs in the consuming internal PR).
• No-deferred path is unchanged: no JSON written, no COPY directives, existing conda tests unaffected.

🤖 Generated with Claude Code

[npow]

Canon PR Review — Engineering Standards Scorecard

PR: Make conda prebuilt-agnostic; restore deepspeed/transnetv2 sdist support (netflixext 1.3.11→1.3.12, prebuilt 0.2.1→0.3.0)
Reviewed against: Top 20 Tier 2 practices (Code Complete, Clean Code, APoSD, Refactoring, Release It!, DDIA, The Pragmatic Programmer, Working Effectively with Legacy Code, Design Patterns GoF)

Scores

Dimension	Score	Gate
Code Quality (naming, SRP, DRY, guard clauses, smells)	4/5	✅
Module Design (deep modules, composition, docs, encapsulation)	4/5	✅
Reliability (error handling, idempotency, circuit breakers, timeouts, degradation)	4/5	✅
Maintainability (small steps, test before change, broken windows, regression tests)	3/5	✅

Decision: APPROVE

---

Violations Found

Reliability

CC-058 — Explicit Timeouts on All Network/Subprocess Calls (Source: Release It!, DDIA)

• Location: metaflow-prebuilt/metaflow_extensions/prebuilt/plugins/conda/prebuilt_build_install.py — the three subprocess.run() calls in _run_deferred_sdist_builds
• Issue: subprocess.run() without timeout= can hang indefinitely inside a Docker build layer if uv/pip stalls (network, package index outage, hanging build subprocess). The build service may have an outer timeout, but an explicit per-subprocess timeout would make the failure mode visible rather than relying on the outer build kill.
• Violation excerpt:

  check_result = subprocess.run(
      [python_bin, "-c", _SETUPTOOLS_CHECK],
      capture_output=True,
      text=True,
  )

  result = subprocess.run(
      build_args, capture_output=True, text=True, env=build_env
  )

• Suggested improvement:

  result = subprocess.run(
      build_args, capture_output=True, text=True, env=build_env,
      timeout=600,  # 10 min hard ceiling per sdist build
  )

  Wrap in subprocess.TimeoutExpired → CondaException to get a useful error message.

Maintainability

CC-093 — Fix Broken Windows: TODOs Without Issue Tracker References (Source: The Pragmatic Programmer)

• Location: prebuilt_conda_environment.py (_image_cache_key, _build_prebuilt_images), buildx_service.py, ecr_registry.py
• Issue: Five TODO comments are added under the image-identity and codebuild-region labels, which function as informal tags but have no issue tracker references. "Pre-existing (inline has it too)" mitigates severity, but the follow-up items are real load-bearing limitations (CPU/GPU base collision, cross-arch deploy failure). Without an issue reference they risk staying permanently deferred.
• Violation excerpt:

  # TODO(image-identity): keys only on req_id/full_id, so steps that share deps
  # but need different images collide on one cached image / registry tag:

• Suggested improvement: Link each TODO to a tracked issue (e.g. TODO(image-identity): github.com/Netflix/metaflow-nflx-extensions/issues/NN) so the follow-up has a clear owner and won't be forgotten across PRs.

---

What's Working Well

1. _make_envs_resolver() factory hook (conda_environment.py, prebuilt_conda_environment.py) — a textbook deep-module extension point. Subclasses inject resolver behavior (defer deferral flag) without duplicating the 100-line init_environment body. Clean.

2. Regression test naming and structure — test_gather_embedded_wheels_prefers_cached_wheel_over_local_sdist is labeled as a "Codex r13 regression," the scenario is precisely described, and the fake_lazy_fetch captures what was actually fetched. This is exactly what a regression test should look like.

3. Deferred-builds hand-off design — always writing deferred_builds.json into the build context even when empty, and always COPY-ing it, correctly prevents stale inheritance from a base image. The comment explaining why is correct and load-bearing.

4. isinstance(environment, CondaEnvironment) widening — replacing the string TYPE != "conda" checks is the right fix: it's strictly more correct, covers subclass polymorphism, and doesn't require a TYPE override in the subclass.

---

Scored against: CC-003, CC-005, CC-006, CC-011, CC-021 (Code Quality) | CC-027, CC-015, CC-028, CC-017 (Module Design) | CC-014, CC-048, CC-056, CC-058, CC-060 (Reliability) | CC-019, CC-031, CC-093, CC-107 (Maintainability)

[npow]

Canon PR Review — Engineering Standards Scorecard

PR: Generic sdist-build hooks in conda; port prebuilt sdist/git support
Reviewed against: Top 20 Tier 2 practices (Code Complete, Clean Code, APoSD, Refactoring, Release It!, DDIA, The Pragmatic Programmer, Working Effectively with Legacy Code, Design Patterns GoF)

Scores

Dimension	Score	Gate
Code Quality (naming, SRP, DRY, guard clauses, smells)	3/5	✅
Module Design (deep modules, composition, docs, encapsulation)	4/5	✅
Reliability (error handling, idempotency, circuit breakers, timeouts, degradation)	4/5	✅
Maintainability (small steps, test before change, broken windows, regression tests)	4/5	✅

Decision: APPROVE

---

Violations Found

Code Quality

CC-021 — Long Method (Source: Refactoring, Clean Code)

• Location: metaflow-prebuilt/metaflow_extensions/prebuilt/plugins/conda/prebuilt_build_install.py — _run_deferred_sdist_builds
• Issue: The function is ~200 lines of body spanning setuptools check, overlay management (ensure_overlay + overlay_install closures), local artifact map construction, and the per-sdist build loop. The inner closures _ensure_overlay and _overlay_install help, but the outer function still does four distinct things. Not a blocker — the logic is coherent and well-commented — but a future extract of _build_sdist_with_overlay(sdist, env_dir, ...) would isolate the per-sdist concern from the per-image overlay lifecycle.
• Violation excerpt:

  def _run_deferred_sdist_builds(env_dir, resolved_env):
      ...
      build_env = dict(os.environ)
      overlay_state = {"dir": None, "staged": {}}
  
      def _ensure_overlay(): ...
      def _overlay_install(pkgs, what): ...
  
      try:
          # (a) setuptools pre-check (b) local artifact map (c) per-sdist loop
          ...
      finally:
          if overlay_state["dir"]:
              shutil.rmtree(overlay_state["dir"], ignore_errors=True)

• Suggested improvement: Extract a _build_one_sdist(sdist, python_bin, uv_bin, build_env, resolved_env, overlay_install) helper so the outer function owns only the overlay lifecycle and the iteration, not the per-sdist build mechanics.

CC-006 — Minor knowledge duplication (Source: The Pragmatic Programmer)

• Location: prebuilt_build_install.py:889-913 (_overlay_install) and prebuilt_build_install.py:985-1013 (Pass B main build)
• Issue: Both branches produce a uv/pip argument list with very similar structure (uv: uv pip install --python <py> ..., pip: python -m pip install ...). The commit message mentions "_install_args helper" was considered — the two call-sites have different flags (--target overlay vs --prefix env_dir, --no-build-isolation --no-deps only in Pass B) so they're not a straight DRY violation, but a shared _make_install_cmd(python_bin, uv_bin, ...) factory accepting target/prefix/extra_flags would remove the conceptual duplication cleanly.
• Severity: Minor — different flags make these distinct enough that the duplication is defensible. Score impact minimal.

Reliability

CC-058 — Missing explicit timeouts on subprocess calls (Source: Release It!)

• Location: prebuilt_build_install.py — three subprocess.run call sites (setuptools probe, _overlay_install, Pass B main build)
• Issue: All three subprocess.run calls have capture_output=True but no timeout= parameter. A pip/uv install that hangs (network stall, deadlocked subprocess) will block the thread indefinitely inside a Docker build step. For build operations this is often accepted practice, but an explicit large timeout (e.g. timeout=3600) with a clear message would surface hangs rather than silently stalling image builds.
• Violation excerpt:

  r = subprocess.run(args, capture_output=True, text=True)
  ...
  result = subprocess.run(build_args, capture_output=True, text=True, env=build_env)

• Suggested improvement:

  BUILD_TIMEOUT_SECS = int(os.environ.get("METAFLOW_PREBUILT_BUILD_TIMEOUT", "3600"))
  result = subprocess.run(build_args, capture_output=True, text=True,
                          env=build_env, timeout=BUILD_TIMEOUT_SECS)

• Severity: Low — build operations are legitimately long-running. Does not change the APPROVE decision.

---

What's Working Well

1. Exemplary deep-module design: _make_envs_resolver() zero-arg factory, only_binary=False default, defer_pypi_sdist_build=False default — all three primitives are default-off, composable, and hide real complexity behind a one-parameter call. The PR description's claim that "non-prebuilt resolve/create is byte-for-byte unchanged" is substantiated by the diff.

2. Regression test discipline: Every bug found in the 17-round review cycle has a targeted regression test that fails on the old code and passes on the fix (test_gather_embedded_wheels_prefers_cached_wheel_over_local_sdist, TestImageIdentity, ensure_remote_conda_test.py, decorator_env_compat_test.py, test_prebuilt_passb.py). This is CC-107 at its best.

3. Comment quality on invariants: The storage-guard comments (# Storage guard: without a datastore...), the mode-guard comment (# We honor an explicitly-set local file even in mode="remote"), and the build-overlay design (# never installed into the env, so the shipped image stays byte-identical to the resolved conda manifest) are exactly the kind of non-obvious constraint documentation CC-028 asks for.

---

To Unblock

Nothing blocking — this is an APPROVE. The two suggestions above (extract a per-sdist helper from _run_deferred_sdist_builds, add subprocess timeouts) are quality improvements worth tracking but not merge blockers.

---

Scored against: CC-003, CC-005, CC-006, CC-011, CC-021 (Code Quality) | CC-027, CC-015, CC-028, CC-017 (Module Design) | CC-014, CC-048, CC-056, CC-058, CC-060 (Reliability) | CC-019, CC-031, CC-093, CC-107 (Maintainability)

[npow]

Canon PR Review — Engineering Standards Scorecard

PR: generic conda sdist-build-deferral hooks + metaflow-prebuilt orchestration package (v0.3.0)
Reviewed against: Top 20 Tier 2 practices (Code Complete, Clean Code, APoSD, Refactoring, Release It!, DDIA, The Pragmatic Programmer, Working Effectively with Legacy Code, Design Patterns GoF)

Scores

Dimension	Score	Gate
Code Quality (naming, SRP, DRY, guard clauses, smells)	3/5	✅
Module Design (deep modules, composition, docs, encapsulation)	4/5	✅
Reliability (error handling, idempotency, circuit breakers, timeouts, degradation)	4/5	✅
Maintainability (small steps, test before change, broken windows, regression tests)	4/5	✅

Decision: APPROVE

---

Violations Found

Code Quality

CC-021 — Long Method (Source: Refactoring, Clean Code)

• Location: metaflow-prebuilt/metaflow_extensions/prebuilt/plugins/conda/prebuilt_build_install.py — _run_deferred_sdist_builds
• Issue: The function is ~150 lines (excluding docstring), well past the 50-line threshold. It handles three distinct sub-phases: (1) the setuptools<82 pre-check and overlay staging, (2) local-file resolution from resolved_env.packages, and (3) the per-sdist build loop. The nested closures (_ensure_overlay, _overlay_install) do help, but they live inside the function body and add to its cognitive load.
• Suggested improvement: Extract _ensure_build_overlay(build_env, python_bin, uv_bin) as a standalone helper that manages the overlay lifecycle and returns a context manager or a staging callable. This keeps the try/finally overlay cleanup contained and reduces _run_deferred_sdist_builds to the happy-path loop + local-file resolution.

Reliability

CC-058 — No explicit timeouts on subprocess calls (Source: Release It!, DDIA)

• Location: prebuilt_build_install.py:_overlay_install and the per-sdist subprocess.run in _run_deferred_sdist_builds
• Issue: Every subprocess.run in the Pass B build path uses capture_output=True, text=True with no timeout=. A stuck uv pip install (network stall, lock contention, or infinite build loop for a pathological sdist) will hang the container's RUN step indefinitely — Docker's build timeout is the only backstop, and that's typically 1 hour.
• Violation excerpt:

  r = subprocess.run(args, capture_output=True, text=True)  # _overlay_install
  result = subprocess.run(build_args, capture_output=True, text=True, env=build_env)  # sdist build

• Suggested improvement: Add a reasonable wall-clock timeout (e.g. 600s for a staging install, 1800s for an sdist compile — or make it configurable via an env var). The failure message already includes r.stderr, so the timeout exception message would be surfaced the same way:

  r = subprocess.run(args, capture_output=True, text=True, timeout=600)

  A subprocess.TimeoutExpired should be caught and re-raised as CondaException with the partial output.

---

What's Working Well

1. CC-028 (Interface comments): Documentation quality throughout is exceptional. _gather_embedded_wheels documents all three cases (B/D/E) with exact conditions. _run_deferred_sdist_builds explains --no-build-isolation rationale, the build overlay lifetime, and residual limitations. _image_variant / _env_cache_key / _image_dedup_key all explain why the split exists and what each key dimension covers — a future maintainer can reason about the invariants without reading the implementation.

2. CC-017 (Encapsulate what varies): The _CONDA_ARCH_TO_DOCKER_PLATFORM dict isolates cross-arch platform mapping in data (adding a new Linux arch is a one-liner). defer_pypi_sdist_build is threaded through a factory hook, keeping the prebuilt-specific opt-in out of core conda. _SCHEMA_VERSION = "2" as a named constant makes schema evolution explicit.

3. CC-107 (Regression tests): Every bug fixed in the 27-commit history has a precise reproduction test that would fail on main and pass with the fix: ensure_remote_conda_test.py pins the truthy-vs-is not None edge, decorator_env_compat_test.py pins the isinstance widening, TestImageIdentity.test_bootstrap_resolves_env_when_images_keyed_by_variant pins the CPU/GPU collision fix. test_prebuilt_passb.py is particularly thorough — it manually constructs a tarball to test the pyproject build-requires extraction path without requiring a real build backend.

---

Scored against: CC-003, CC-005, CC-006, CC-011, CC-021 (Code Quality) | CC-027, CC-015, CC-028, CC-017 (Module Design) | CC-014, CC-048, CC-056, CC-058, CC-060 (Reliability) | CC-019, CC-031, CC-093, CC-107 (Maintainability)