microsoft / microsoft/amplifier
Resolve depends_on half-enforcement in amplifier-bundle-recipes — pick A (advisory) or B (enforced)
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3.1k
- Forks
- 261
- Avg merge
- 3h 28m
- Merged PRs (30d)
- 13
Description
Filed in this repo because
microsoft/amplifier-bundle-recipeshas GitHub Issues disabled. The architecture question concerns the recipe engine inamplifier-bundle-recipes; please route to the recipe engine maintainers.
Background
Step.depends_on is currently half-enforced in the recipe engine.
- The validator (
validator.py:362-383,check_step_dependencies) rejects recipes where anydepends_ontarget appears after its dependent step — load-time ERROR. - The executor (
executor.py:710-733) iterates steps in strict declaration order viafor i in range(...)and never consultsdepends_onat runtime.
The just-merged amplifier-bundle-recipes#76 (fix: collapse per-step depends_on warning to one per recipe) collapsed the per-step warning storm into a single per-recipe warning. That fixes the noise but leaves the underlying design question unresolved.
The three options
Option A — depends_on becomes advisory only
Delete the validator's ordering check. Document depends_on as advisory in the recipe schema. Possibly rename or deprecate the field. Keep the existence check.
- Pros: honest; zero runtime change; removes conceptual debt; the per-recipe warning becomes the on-ramp to a future enforced version if one ever comes.
- Cons: some recipe authors expected real semantics; they lose a guardrail.
- Effort: ~1 day, one PR.
Option B — depends_on becomes enforced at execution time
Replace declaration-order iteration with topological-order iteration. Add cycle detection. Keep the validator check. Delete the warning. Optional follow-up: parallel execution for independent steps.
- Pros: the field means what it says; enables future parallelism; aligns with Make, Airflow, Argo, GitHub Actions
needs:, Prefect. - Cons: behavior change for every existing recipe; subtle break when a step relying on "B runs after A because declared after A" without saying so encounters a conditional skip; topological-sort failure modes; needs regression tests across foundation's recipes; invalidates the "execute in declaration order" mental model.
- Effort: multi-week design + implementation, needs its own design doc first.
Option C — Status quo, better signaling
What amplifier-bundle-recipes#76 just shipped. One warning per recipe. Field remains half-enforced.
- Pros: zero further work.
- Cons: solves nothing structurally; the next maintainer revisits the same question.
Recommendation
Pick A. Plan for B only if/when parallel execution becomes valuable.
Sequencing:
- Recipe engine maintainers comment with A / B / C / other.
- If A: one PR (~1 day) — delete validator ordering check, update schema docs, optionally remove the per-recipe warning shipped in amplifier-bundle-recipes#76.
- If B: design doc first at
docs/designs/depends-on-enforcement.md, then implementation PR(s). - If C: close with rationale recorded.
Why A over B (briefly)
The engine has shipped, recipes exist, and "execute in declaration order" is the de-facto contract. Breaking it for a feature no current recipe relies on (parallelism, conditional-aware scheduling) is a bad trade. Option A's cost is roughly one PR. Option B's cost is a feature with a migration window. They are not the same shape of work.
If Option B becomes valuable later, Option A doesn't block it — you re-add the field's runtime semantics with full design discipline at that point. Option C blocks it by accumulating recipes built on ambiguous semantics.
References
- Make's dependency model: https://www.gnu.org/software/make/manual/make.html#Rules
- GitHub Actions
needs:: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds - Argo Workflows DAG: https://argo-workflows.readthedocs.io/en/latest/walk-through/dag/
Context
amplifier-bundle-recipes#76 introduced the per-recipe warning. The commit author of 8dae711 ("three COE-mandated fixes...") added the original per-step warning intentionally — they may have opinions on A vs B that should be captured here.
cc: please retag/relabel/transfer as appropriate for your team's design-tracking flow.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with validator.py:362-383 and executor.py:710-733 to understand the current dependency validation and declaration-order execution. Before changing code, obtain a maintainer decision between advisory, enforced, or status-quo behavior. Done means the selected design is documented and its implementation or closure path is agreed, with tests and recipe impacts identified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100