Specify read-write symmetry exception for backup replicas in P4Runtime Spec

[matthewtlam]

@smolkaj @chrispsommers @jafingerhut

[chrispsommers]

LGtM

[jafingerhut]

LGTM

[matthewtlam]

@jafingerhut @chrispsommers I was wondering if you knew what these checks were and why they are waiting for the status to be reported?

I don't believe that my changes have anything breaking and I was hoping to submit the PR

[chrispsommers]

@matthewtlam I'm not sure, perhaps because uploads are not done on PRs, only merges to main? It looks like we maintainers can bypass the warning but I'd rather understand this first. @jafingerhut or @smolkaj any ideas?

[jafingerhut]

Those names main-branch-uploads and any-branch-uploads definitely correspond to names of workflows in this directory: https://github.com/p4lang/p4runtime/tree/main/.github/workflows

I can tell from the Settings on this Github repo that they are listed as required things for the main branch.

I do not know the history of those CI tasks, or why they are required.

[jafingerhut]

From git commit log, main-branch-workflows.yml was created in 2022 by Antonin Bas, when this repo transitioned from using Travis CI to using Github Actions, then it was tweaked slightly in 2025 by Davide Scano related to the Madoko -> AsciiDoc transition.

[jafingerhut]

Given that an inconsequential change to the spec and creating a PR from it has the same effect (see #549), that is strong evidence that the hung tests have nothing to do with this PR's changes.

I am not sure, but perhaps someone marked these 2 CI tests as requirements for PRs, but they should not have? I do not know of any commit log for Github settings, unfortunately, so I have no easy way to check this other than asking folks who might have changed it.

@antoninbas @fruffy @Dscano Do any of you know if these 2 CI tests that are "Waiting for status to be reported", perhaps forever, should be changed in Github settings so they are not required for a PR to be merged?

Aside: And it seems almost software-engineering criminal that folks working at Github did not think to make the history of all settings on a repo available via git log on some configuration file!

[smolkaj]

I did have to update the "required checks" as part of #521, since that changed the name of the bazel CI runs. I did not intend to make changes to the upload checks, but I wouldn't rule out that I did make such changes accidentally.

[smolkaj]

+1 on figuring out if the "upload" CI checks are intended to be required or not.

[fruffy]

+1 on figuring out if the "upload" CI checks are intended to be required or not.

You should be able to set the required CI checks in Settings->Branches->Main->Required Status Checks. The available checks are determined by recent PRs that run. Outdated checks have to be removed/replaced.

Edit: Oh I see the issue. These workflows are not being triggered properly. It is possible that the trigger setting here is not activated because @matthewtlam is pushing to a branch on a fork. What is missing is the pull_request triggered like here. But this is just guesswork.

[jafingerhut]

+1 on figuring out if the "upload" CI checks are intended to be required or not.

You should be able to set the required CI checks in Settings->Branches->Main->Required Status Checks. The available checks are determined by recent PRs that run. Outdated checks have to be removed/replaced.

Edit: Oh I see the issue. These workflows are not being triggered properly. It is possible that the trigger setting here is not activated because @matthewtlam is pushing to a branch on a fork. What is missing is the pull_request triggered like here. But this is just guesswork.

Pushing to a branch on a fork is a common workflow on Github, one that we should consider normal, I think.

Pushing to a branch in the repo where you want to create the PR is also fairly common, but less common, since you have to have permission to create branches in the repo, which we don't give everyone permission to do.

In short, @chrispsommers, if you think this PR's changes are good, I'd say hit the check box to override the warning and merge it, if you approve and are ready to merge it. This kerfuffle with weird required CI tasks that should not be blocking the issue are a temporary solvable thing that we will solve, but probably not today. And given that I have already reproduced this particular behavior on another simple PR that was also created from a branch on a fork, we know how to reproduce this scenario pretty easily, and do not need this particular PR to wait in order to diagnose the issue.

[chrispsommers]

@jafingerhut @fruffy @smolkaj thanks for weighing in. Andy I agree. I am inclined to merge. Any objections?

[smolkaj]

+1 on merging + opening a separate issue to track fixing the CI issue.

EDIT: separate issue is no longer needed.

[smolkaj]

Taking another close look at any-branch-uploads.yml and main-branch-uploads.yml and comparing to our other CI files, it seems clear that they are very much not intended to be run for pull requests, only for branch pushes.

on:
  push:
    branches:
      - main

Thus removing them from the list of required checks.

I am now 90% sure I introduced this regression, see #548 (comment). My bad!