aglinxinyuan opened a new pull request, #5047:
URL: https://github.com/apache/texera/pull/5047

   ### What changes were proposed in this PR?
   Replace the raw-payload `Merge Queue` ruleset added in #5036 with a 
convenience-syntax `Main branch protection` ruleset so ASF Infra stops emailing 
validation errors on every push to `main`.
   
   ### Root cause
   Since #5036 merged, every push to `main` triggers this email from ASF Infra:
   
   ```
   Validation failed while creating ruleset 'Merge Queue': Invalid request.
   
   Invalid property /bypass_actors/0/actor_id: `"4"` is not of type `integer`.
   Invalid property /rules/2: data matches no possible input.
   Invalid property /rules/3: data matches no possible input.
   Invalid property /rules/5: data matches no possible input.
   ```
   
   `asfyaml` loads `.asf.yaml` via 
[`strictyaml.dirty_load`](https://github.com/apache/infrastructure-asfyaml/blob/main/asfyaml/asfyaml.py),
 which leaves **every scalar as a string** (this is documented strictyaml 
behavior). The convenience-syntax path in 
[`rulesets.py`](https://github.com/apache/infrastructure-asfyaml/blob/main/asfyaml/feature/github/rulesets.py)
 coerces values back with `_expect_bool` / `_expect_int` before calling the 
GitHub API. The **raw-payload path** (`rules:` / `bypass_actors:`) just does 
`dict(ruleset)` and passes the stringified data straight to GitHub.
   
   Reproduced the stringification with strictyaml against the current `main`:
   
   | YAML source | type before serialization | what GitHub receives |
   |---|---|---|
   | `actor_id: 4` | `str` | `"4"` |
   | `max_entries_to_build: 5` | `str` | `"5"` |
   | `dismiss_stale_reviews_on_push: false` | `str` | `"false"` |
   | `required_approving_review_count: 1` | `str` | `"1"` |
   | `strict_required_status_checks_policy: false` | `str` | `"false"` |
   | `integration_id: -1` | `str` | `"-1"` |
   
   GitHub's Rulesets API rejects strings where it expects `integer`/`boolean`. 
That matches the four `Invalid property` lines (the parameter-less rules 
`deletion`, `non_fast_forward`, `required_linear_history` are accepted because 
they have no typed parameters to fail on).
   
   ### Fix
   Convert the ruleset to **convenience syntax**, which routes through 
asfyaml's type-coercion helpers. Same protections as #5036 intended, minus the 
`merge_queue` rule (asfyaml's convenience syntax does not yet expose it).
   
   ```
   before (#5036)                            after (this PR)
   ──────────────────────────                ──────────────────────────
   meta.environment: github_rulesets         meta.environment: github_rulesets  
(unchanged)
   github.rulesets:                          github.rulesets:
     - name: Merge Queue                       - name: "Main branch protection"
       target / conditions / rules (raw)        type / branches / required_*  
(convenience)
       rules:                                   required_pull_request_reviews:
         - deletion                               
required_approving_review_count: 1
         - non_fast_forward                       dismiss_stale_reviews: false
         - merge_queue       ← dropped            require_code_owner_reviews: 
false
         - pull_request                           require_last_push_approval: 
false
         - required_linear_history              
required_conversation_resolution: true
         - required_status_checks               required_linear_history: true
       bypass_actors:        ← dropped          required_status_checks:
         - actor_id: 4                            - "Required Checks"
           actor_type: RepositoryRole             - "Check License Headers"
           bypass_mode: always                    - "Validate PR title"
                                                (deletion + non_fast_forward 
implied by
                                                 restrict_deletion / 
restrict_force_push
                                                 defaulting to true)
   ```
   
   | Protection from #5036 | Preserved? |
   |---|---|
   | 1 approving review | ✅ via `required_approving_review_count: 1` |
   | Conversation resolution | ✅ via `required_conversation_resolution: true` |
   | Linear history | ✅ via `required_linear_history: true` |
   | Required Checks / Check License Headers / Validate PR title | ✅ via 
`required_status_checks:` list |
   | `strict` disabled (no force-update-before-merge) | ✅ defaults to false |
   | Block deletion | ✅ default `restrict_deletion: true` adds `deletion` rule |
   | Block force push | ✅ default `restrict_force_push: true` adds 
`non_fast_forward` rule |
   | `merge_queue` rule | ❌ dropped — see below |
   | `bypass_actors` for Maintain role | ❌ dropped — convenience syntax only 
exposes `bypass_teams` |
   
   ### What this PR does NOT do
   - **No GitHub merge queue**. asfyaml does not currently support the 
`merge_queue` rule in convenience syntax, and the raw payload path is broken as 
shown above. A follow-up issue should track re-enabling merge queue once 
asfyaml adds support (or properly types raw payload values). The `merge_group` 
workflow trigger added in #5036 stays in place — it's a no-op without an active 
merge queue and costs nothing.
   - **No revert of `.github/workflows/auto-queue.yml`**. That removal was 
intentional in #5036 and unrelated to the validation errors.
   
   ### How was this PR tested?
   Re-implemented asfyaml's `_to_payload_ruleset` locally against this branch's 
`.asf.yaml` and verified every parameter the GitHub API cares about has the 
expected type:
   
   ```bash
   python -c "
   import strictyaml, json, pathlib
   data = strictyaml.dirty_load(pathlib.Path('.asf.yaml').read_text(), 
allow_flow_style=True).data
   # ...runs asfyaml's coercion logic, builds the same payload asfyaml POSTs to 
/repos/.../rulesets...
   "
   ```
   
   All `bool` / `int` fields come out with the correct types; `integration_id: 
-1` lands as `int(-1)`, not `str("-1")`. Also re-ran the same test against 
`origin/main` to confirm the broken-types diagnosis above.
   
   No runtime tests added — this PR only changes repository configuration.
   
   ### Any related issues, documentation, discussions?
   Follow-up to #5036. The merge-queue regression should be tracked separately 
and re-enabled when 
[apache/infrastructure-asfyaml](https://github.com/apache/infrastructure-asfyaml)
 supports `merge_queue` (either by extending the convenience syntax or by 
typing values in the raw payload path before POSTing to GitHub).
   
   ### Was this PR authored or co-authored using generative AI tooling?
   Generated-by: Claude Code (Opus 4.7)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to