-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update-pr-preview often failing #19378
Comments
@jugglinmike we've already decided to improve the visibility of the the deployment in Q4 which will involve touching this job, so assigning to you. If it keeps failing like this, since it's not possible to discover where it's been deployed to even when successful, then I'd like to disable it until that work starts. |
It appears that the permissions model of the GitHub Actions feature has changed and that the new implementation is fundamentally incompatible with the solution we designed. I believe we'll need to re-implement the "git tag" and "Pull Request label" operations on an external server. Comparing the pull requests which have received the When initially implemented, we demonstrated that this feature would work for contributors that do not have "write" access to the repository. About three weeks ago, a GitHub employee wrote:
The documentation referenced in our script no longer exists, and the new documentation states:
Which leads me to believe that we'll need to grant the necessary permissions to an external service and re-write the GitHub Action to notify that service. This is a non-trivial amount of work, though, so I'd appreciate it if @foolip or any of the other infrastructure maintainers could verify my interpretation! |
As discovered in #19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup.
A token with read-only access to the target repository makes good sense for PRs from forks, and I can understand why GitHub changes it to work like that. Without write access we could still deploy (by polling open pull requests) but can't in any way point to it from the pull request. I agree it seems like we need to have an external service doing the commenting/tagging/whatever. Pending a new solution I've sent #19456, since it likely won't remain in that form and it's not working as intended. |
Whether we post comments or use GitHub's deployments API I'm inclined to say that the same code that monitors for open PRs in need of deployment would also post back when the deployment is done. That there are multiple instances for load balancing probably complicates this somewhat, but it has to be possible to answer the question "will the URL load" somehow. |
As discovered in #19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup.
GitHub Actions supports triggering Workflow events on a pre-defined schedule. A quick experiment on the wpt-actions-test project proved that the token supplied to these executions has write permissions. By querying the GitHub API, we could implement issue labeling and commit tagging completely in-repository. From the perspective of maintenance and simplicity, this seems preferable to the solution I suggested above. Some risks come to mind, though.
|
Rather than mirroring pull requests from forks automatically, perhaps we can trigger it by a special label being added and reacting to that event? Can you check if the token for those actions has write access? |
The documentation for the "Pull Request Event" in GitHub Actions has the following note about "pull request" Events for forked repositories:
The referenced document shows that "Access by forked repos" is read-only for all available Events. |
Alright, I guess that does make sense, it'd be a more complicated model to reason about if what I suggested did work. Since the workflow no longer exists I'm going to close this issue. @jugglinmike can you file another, if there isn't one already, for getting deployments to wptpr.live working and reported on PRs? We can discuss there what the tradeoffs are, and I'd like to get @Hexcles's feedback there as well. |
Sure! See gh-19607. |
…eview job, a=testonly Automatic update from web-platform-tests [GitHub Actions] remove the update-pr-preview job (#19456) As discovered in web-platform-tests/wpt#19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup. -- wpt-commits: 81b79d799e8f87639725ef965ac16b436b7498bb wpt-pr: 19456
…eview job, a=testonly Automatic update from web-platform-tests [GitHub Actions] remove the update-pr-preview job (#19456) As discovered in web-platform-tests/wpt#19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup. -- wpt-commits: 81b79d799e8f87639725ef965ac16b436b7498bb wpt-pr: 19456
…eview job, a=testonly Automatic update from web-platform-tests [GitHub Actions] remove the update-pr-preview job (#19456) As discovered in web-platform-tests/wpt#19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup. -- wpt-commits: 81b79d799e8f87639725ef965ac16b436b7498bb wpt-pr: 19456 UltraBlame original commit: 5c670cc39b282dd3ca862302210f3da89fda10cf
…eview job, a=testonly Automatic update from web-platform-tests [GitHub Actions] remove the update-pr-preview job (#19456) As discovered in web-platform-tests/wpt#19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup. -- wpt-commits: 81b79d799e8f87639725ef965ac16b436b7498bb wpt-pr: 19456 UltraBlame original commit: 5c670cc39b282dd3ca862302210f3da89fda10cf
…eview job, a=testonly Automatic update from web-platform-tests [GitHub Actions] remove the update-pr-preview job (#19456) As discovered in web-platform-tests/wpt#19378, the existing setup isn't compatible with the GitHub Actions read-only tokens for pull requests from forks. Since the location of the staging deployment isn't very discoverable even when it does work, disable the job pending a new setup. -- wpt-commits: 81b79d799e8f87639725ef965ac16b436b7498bb wpt-pr: 19456 UltraBlame original commit: 5c670cc39b282dd3ca862302210f3da89fda10cf
The "Synchronize the Pull Request Preview / update-pr-preview" check has been failing on many pull requests lately:
#19048
#19209 (comment)
#19323 (comment)
#19370 (comment)
All of them fail like in https://github.com/web-platform-tests/wpt/pull/19048/checks?check_run_id=221550321:
The text was updated successfully, but these errors were encountered: