Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make manifest releases almost atomic using draft releases #19593

Merged
merged 3 commits into from Oct 9, 2019
Merged

Make manifest releases almost atomic using draft releases #19593

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41daef7
Let `create_release` implicitly create the merge_pr_* tag
foolip Oct 9, 2019
01e7914
Let `create_release` publish only after uploading assets
foolip Oct 9, 2019
06ba828
Use PATCH method for editing a release
foolip Oct 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 16 additions & 25 deletions tools/ci/manifest_build.py
View file
Open in desktop
Expand Up @@ -105,32 +105,19 @@ def get_pr(owner, repo, sha):
return pr["number"]


def tag(owner, repo, sha, tag):
data = {"ref": "refs/tags/%s" % tag,
"sha": sha}
url = "https://api.github.com/repos/%s/%s/git/refs" % (owner, repo)

resp_data = request(url, "Tag creation", json_data=data)
if not resp_data:
return False

logger.info("Tagged %s as %s" % (sha, tag))
return True


def create_release(manifest_path, owner, repo, sha, tag, body):
create_url = "https://api.github.com/repos/%s/%s/releases" % (owner, repo)
create_data = {"tag_name": tag,
"target_commitish": sha,
"name": tag,
"body": body}
create_data = request(create_url, "Release creation", json_data=create_data)
if not create_data:
"body": body,
"draft": True}
create_resp = request(create_url, "Release creation", json_data=create_data)
if not create_resp:
return False

# Upload URL contains '{?name,label}' at the end which we want to remove
upload_url = create_data["upload_url"].split("{", 1)[0]

success = True
upload_url = create_resp["upload_url"].split("{", 1)[0]

upload_exts = [".gz", ".bz2", ".zst"]
for upload_ext in upload_exts:
Expand All @@ -146,9 +133,17 @@ def create_release(manifest_path, owner, repo, sha, tag, body):
upload_resp = request(upload_url, "Manifest upload", data=upload_data, params=params,
headers={'Content-Type': 'application/octet-stream'})
if not upload_resp:
success = False
return False

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this will do a POST request not PATCH?

Copy link
Member Author

@foolip foolip Oct 9, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding a method argument that defaults to None is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if not edit_resp:
return False

return success
logger.info("Released %s" % edit_resp["html_url"])
return True


def should_dry_run():
Expand Down Expand Up @@ -190,10 +185,6 @@ def main():
return Status.FAIL
tag_name = "merge_pr_%s" % pr

tagged = tag(owner, repo, head_rev, tag_name)
if not tagged:
return Status.FAIL

if not create_release(manifest_path, owner, repo, head_rev, tag_name, body):
return Status.FAIL

Expand Down