Chaosthebot / Chaos

A social coding experiment that updates its own code democratically.
http://chaosthebot.com
MIT License
2.44k stars 210 forks source link

Ensure labels creation #451

Closed Swizz closed 7 years ago

Swizz commented 7 years ago

Repo labels could be deleted any time.

So could we attempt to create them at startup (like description) ?

(If a label name already exist, the api call will be silently ignored)

edit : just seen I shipped 27b8b65 from #435 is this a problem with our squash enabled strategy ?

phil-r commented 7 years ago

@Swizz What happens if label already exists? it's just ignored?

Swizz commented 7 years ago

Github API return 422 HTTP Code with

{
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "Label",
      "code": "already_exists",
      "field": "name"
    }
  ],
  "documentation_url": "https://developer.github.com/v3/issues/labels/#create-a-label"
}
Swizz commented 7 years ago

I previously experimented, label deletion, but I think this isn't a good compatibility idea. We want here to create new labels and ensure they are still here, deleting labels will remove labels on all PR/Issue they were.

Notes :

chaosbot commented 7 years ago

:ok_woman: PR passed with a vote of 13 for and 0 against, a weighted total of 12.5 and a threshold of 6.5, and a current meritocracy review.

See merge-commit 651c90a73de145128b9238a7c36613a2d02d117a for more details.

Swizz commented 7 years ago

Erf, 422 seem to not fail silently with request.

06-01 11:46 urllib3.connectionpool DEBUG    https://api.github.com:443 "POST /repos/chaosbot/Chaos/labels HTTP/1.1" 422 186
Traceback (most recent call last):
  File "chaos.py", line 120, in <module>
    main()
  File "chaos.py", line 66, in main
    github_api.repos.create_label(api, settings.URN, label, color)
  File "/root/workspace/Chaos/github_api/repos.py", line 46, in create_label
    return api("post", "/repos/{urn}/labels".format(urn=urn), json=data)
  File "/root/workspace/Chaos/github_api/__init__.py", line 93, in __call__
    resp.raise_for_status()
  File "/root/.virtualenvs/chaos/lib/python3.6/site-packages/requests/models.py", line 929, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 422 Client Error: Unprocessable Entity for url: https://api.github.com/repos/chaosbot/Chaos/labels
hongaar commented 7 years ago

Yikes, crashed the thing. Where's the automatic rollback script when you need it?

Encountered the same in #453, workaround is simple

Swizz commented 7 years ago

Yeah, like I said in my Public Apology #458, I didnt noticed the resp.raise_for_status(), so will need to catch any HTTP code which said "error or something"

Swizz commented 7 years ago

@hongaar This could do the trick or I need to move at github_api level ?

log.info("Ensure creation of issue/PR labels")
for label, color in settings.REPO_LABELS.items():
    try:
        github_api.repos.create_label(api, settings.URN, label, color)
    except HTTPError:
        pass

In other words, who is responsible to catch when label already exists ? The function which call the label creation or the api function that make the HTTP call ?

hongaar commented 7 years ago

@Swizz It'd place try/except in create_label() function:

try:
    return api("post", "/repos/{urn}/labels".format(urn=urn), json=data)
except HTTPError:
    # Label might already exist
    pass
phil-r commented 7 years ago

@Swizz in the function is cleaner IMHO P.S. Ive trusted you... an you haven't even tried it out ;(

Swizz commented 7 years ago

@phil-r Without resp.raise_for_status() it would been silent x)

hongaar commented 7 years ago

Maybe change the implementation to replace_label (analogue to SQL replace), which checks if a label exists first, updates it if it does, and creates it if it doesn't? That way, we can change the label colours programatically!

Swizz commented 7 years ago

At start, I didn't want too much logic, but, I guess it could be great. Only deletion and renaming are against the backward compatibility specs.

So only label creation and color change will be allowed.

amoffat commented 7 years ago

Ok hotfix out, chaos back up.