Make manifest releases almost atomic using draft releases #19593
Conversation
Per https://developer.github.com/v3/repos/releases/#create-a-release, `target_commitish` is used to create the tag, and ignored if the tag already exists.
This makes tagging and releasing almost atomic, at most leaving behind a draft release if something goes wrong. Because no tag or (non-draft) release will be created in case of error, the script can just be run again without first removing the bad release.
|
There are two commit on this branch where the first one would make sense in isolation and for review I recommend reading each commit message: To gain some confidence that this works I make some local edits to the script to release to my fork, and emulated something going wrong when the .zst file was going to be uploaded. Here's the resulting draft release and the successful release once I removed the fake error: I've confirmed that no tag is created for drafts, even though you can't tell from the state of my repo right now. |
tools/ci/manifest_build.py
Outdated
| release_id = create_resp["id"] | ||
| edit_url = "https://api.github.com/repos/%s/%s/releases/%s" % (owner, repo, release_id) | ||
| edit_data = {"draft": False} | ||
| edit_resp = request(edit_url, "Release publishing", json_data=edit_data) |
There was a problem hiding this comment.
It looks like this will do a POST request not PATCH?
There was a problem hiding this comment.
Yep, didn't spot the PATCH in https://developer.github.com/v3/repos/releases/#edit-a-release and it looks like GitHub accepts this.
The easy way to fix this is to add a method=None argument to request, but unless it's a string enum it would leak that fact that requests is being used within.
There was a problem hiding this comment.
I think adding a method argument that defaults to None is fine.
There was a problem hiding this comment.
I've pushed a third commit to do just that, and verified that it could produce https://github.com/foolip/wpt/releases/tag/merge_pr_19493.
No description provided.