Adding ev2 execution constraints level - #331
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new level semantics and the existing OR-evaluated constraints don’t align with the current index-based equality check, risking false “metadata conflict” errors (and omitted vs explicit “Cloud” comparisons) without the suggested normalization.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the pipeline execution-constraints model to support an explicit Ev2 constraint “level” knob (Cloud vs Region), enabling scenarios where a deployment should run once per region across a cloud even when regions aren’t explicitly listed.
Changes:
- Adds
leveltoExecutionConstraint(Go type + YAML/JSON shape). - Updates the v1 pipeline JSON schema to validate
levelas an enum (Cloud/Region). - Updates graph resource-group metadata equality logic to consider the new field and updates testdata pipeline YAML to include
level: Region.
File summaries
| File | Description |
|---|---|
| pipelines/types/resourcegroup.go | Adds ExecutionConstraint.Level and documents intended behavior. |
| pipelines/types/pipeline.schema.v1.json | Adds executionConstraint.level enum validation to schema v1. |
| pipelines/graph/graph.go | Compares Level during resource group metadata conflict detection. |
| config/testdata/pipelines/pipeline.yaml | Adds level: Region to a test pipeline fixture. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| for i, ac := range a.ExecutionConstraints { | ||
| bc := b.ExecutionConstraints[i] | ||
| if ac.Singleton != bc.Singleton { | ||
| return false | ||
| } |
Jonathan CHang (jonathan34c)
left a comment
Jonathan CHang (jonathan34c)
left a comment
There was a problem hiding this comment.
LGTM
Steve Kuznetsov (stevekuznetsov)
left a comment
Steve Kuznetsov (stevekuznetsov)
left a comment
There was a problem hiding this comment.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jonathan34c, rachelvweber, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
0ae0b8b to
cd688c8
Compare
|
New changes are detected. LGTM label has been removed. |
Adding ev2 execution constraints level knob to pipelines. For use when you want to deploy to all regions within a cloud with execution constraints. Previously, if just cloud was specified, the step would run once per cloud.