python / miss-islington Goto Github PK

In python/cpython#5746, two core devs approved the PR initially, but later both reviews were dismissed.
miss-islington still went ahead and merged the PR.

The PR shouldn't have been merged in this case.

When I merged python/cpython#5981 with backport labels, the usual backport PRs were generated and I approved both. python/cpython#5986 for 3.7 went normally. python/cpython#5987 for 3.6 got hung-up waiting for Appveyor to report. I could not check details because there was no details link. Miss I. twice said "Backport status check is done, and it's a success", which usually triggers a merge. To try to restart CI, I closed and opened, but Miss I. reclosed, permanently.

I went back to original PR and re-added 3.6 backport label. python/cpython#5993 then proceeded normally.

I don't know that there is anything to do, except to take notice of a new? failure mode.

Would be nice to be able to trigger the backport after PR has been merged, in case that people forgot to apply the needs backport label.

Some thoughts:

• Allow people to leave a comment like "@miss-islington backport this" or something like that
• Perhaps this ability should be restricted to the PR author or core devs or even core dev who merged the PR. So if a random person or another bot leaves the comment, miss-islington will ignore it.

If needs backport label was added after the PR has been merged, it should trigger the backport.

Automerge feature caused miss-islington to reach rate limit.
Need to figure out what to do about it.

In python/cpython#4003, miss-islington made multiple backport attempts. It should only be done once.

The error can be caught.

Log:
Aug 09 15:03:52 miss-islington app/web.1: GH delivery ID 19e9ae34-9c20-11e8-8119-f84026ce9ae3
Aug 09 15:03:53 miss-islington heroku/router: at=info method=POST path="/" host=miss-islington.herokuapp.com request_id=61fe7df2-985b-4549-bbaf-8a0d34964361 fwd="192.30.252.34" dyno=web.1 connect=1ms service=2394ms status=500 bytes=187 protocol=https
Aug 09 15:03:54 miss-islington app/web.1: Traceback (most recent call last):
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/main.py", line 40, in main
Aug 09 15:03:54 miss-islington app/web.1: await router.dispatch(event, gh)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/.heroku/python/lib/python3.6/site-packages/gidgethub/routing.py", line 80, in dispatch
Aug 09 15:03:54 miss-islington app/web.1: await callback(event, *args, **kwargs)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/status_change.py", line 22, in check_status
Aug 09 15:03:54 miss-islington app/web.1: await check_ci_status_and_approval(gh, sha, leave_comment=True)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/status_change.py", line 80, in check_ci_status_and_approval
Aug 09 15:03:54 miss-islington app/web.1: await merge_pr(gh, pr_number, sha)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/status_change.py", line 96, in merge_pr
Aug 09 15:03:54 miss-islington app/web.1: "merge_method": "squash",
Aug 09 15:03:54 miss-islington app/web.1: File "/app/.heroku/python/lib/python3.6/site-packages/gidgethub/abc.py", line 104, in put
Aug 09 15:03:54 miss-islington app/web.1: data, _ = await self._make_request("PUT", url, url_vars, data, accept)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/.heroku/python/lib/python3.6/site-packages/gidgethub/abc.py", line 66, in _make_request
Aug 09 15:03:54 miss-islington app/web.1: data, self.rate_limit, more = sansio.decipher_response(*response)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/.heroku/python/lib/python3.6/site-packages/gidgethub/sansio.py", line 310, in decipher_response
Aug 09 15:03:54 miss-islington app/web.1: raise exc_type(*args)
Aug 09 15:03:54 miss-islington app/web.1: gidgethub.BadRequest: Base branch was modified. Review and try the merge again.
Aug 09 15:03:54 miss-islington app/web.1: Traceback (most recent call last):
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/main.py", line 40, in main
Aug 09 15:03:54 miss-islington app/web.1: await router.dispatch(event, gh)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/.heroku/python/lib/python3.6/site-packages/gidgethub/routing.py", line 80, in dispatch
Aug 09 15:03:54 miss-islington app/web.1: await callback(event, *args, **kwargs)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/status_change.py", line 30, in pr_reviewed
Aug 09 15:03:54 miss-islington app/web.1: await check_ci_status_and_approval(gh, sha)
Aug 09 15:03:54 miss-islington app/web.1: File "/app/miss_islington/status_change.py", line 80, in check_ci_status_and_approval
Aug 09 15:03:54 miss-islington heroku/router: at=info method=POST path="/" host=miss-islington.herokuapp.com request_id=e469ce6a-3e94-46d5-8a34-8431731e8dfd fwd="192.30.252.34" dyno=web.1 connect=0ms service=2719ms status=500 bytes=187 protocol=https

See python/cpython@50c99d9 for example

Examples:
python/cpython#7394
python/cpython#7386

Seeing 500 errors from the webhooks.

In python/cpython#3066 we never heard back from miss-islington.
It is supposed to leave a comment if a backport PR could not be created.

It seems that the robot face emoji and pick emoji are showing up as boxes when @miss-islington adds Thanks and Sorry comments in PRs. I tested it on latest Firefox nightly and Chrome 67 on Mac OS with both of them showing boxes. The same case with README too.

Thanks

Currently we use custom cherry-pick scripts to backport to other branches (manual). I love the idea of automated back-porting. Some information about how to customize this bot and use it for other repositories would be awesome. I want to understand how can I fork this, customize(I can work on that) and include this in other repositories I own.

Any help is appreciated.

E.g. python/cpython#6048 start out with no bpo in the title. Had this happen as second time earlier today on another set of backports related to https://bugs.python.org/issue32758 .

miss-islington should be able to merge the backport PR after:

• all CI finished and all green
• the PR approved by a core dev

This should only apply to backport PRs made by @miss-islington.

Requested by @vstinner in https://mail.python.org/pipermail/python-committers/2018-January/005174.html

$ git show -s b03623227ed1264e3cac4e6bb4878d96b91aa484 --pretty=fuller
commit b03623227ed1264e3cac4e6bb4878d96b91aa484
Author:     Miss Islington (bot) <[email protected]>
AuthorDate: Thu Sep 7 11:35:03 2017 -0700
Commit:     Benjamin Peterson <[email protected]>
CommitDate: Thu Sep 7 11:35:03 2017 -0700

    [3.6] fixes bpo-31373: fix undefined floating-point demotions (GH-3396) (#3424)
    
    (cherry picked from commit a853a8ba7850381d49b284295dd6f0dc491dbe44)

The author metadata should be from the original commit not stamped over. (e.g., so the Author should still be me in the above example)

I'm not sure if this is actually possible, since GitHub is actually doing the author rewrite during the squash.

In python/cpython#5781 miss-islington made the backport PR, however there was an additional commit by a core dev.

That scenario is not yet handled:

• it did not report the status check result
• it did not auto-merge

When I merged python/cpython#2494 with a backport label, I got this message.

Thanks @wohlganger for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.

An hour later, .... nothing.

I ran cherry_picker the old way and in 10-15 seconds had
python/cpython#3487.

Miss Islington would be even more helpful if she would just tell me the right command. For example python/cpython#3674 (review) has comment

Sorry @felipeochoa and @tiran, I had trouble checking out the 2.7 backport branch.
Please backport using cherry_picker on command line.

She has all the information to tell me the right hash and branch(es):

Sorry @felipeochoa and @tiran, I had trouble checking out the 2.7 backport branch.
Please backport using cherry_picker on command line:

cherry_picker 19e4d93 2.7``.

I just have merged my first PR after miss-islington become working. If the original author and the committer is the same person, miss-islington's comment's look slightly strange.

Thanks @serhiy-storchaka for the PR, and @serhiy-storchaka for merging it

Sorry, @serhiy-storchaka and @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.

This is low priority, but I think that it would be nice to not duplicate the name in this case.

Thank you for your awesome work @Mariatta! Your tools help a lot!

Initially done via #27 for #26.

#26 was asking for this to be done in case of errors, but it gets done always.

This caused like 4+ notifications due to a PR being backported to 3.7 and 3.8.

Ref: python/cpython#16066 (comment)

Miss-islington should leave a comment if merging failed, e.g. if it doesn't have the right permission to merge into protected branch.
At the moment it fails silently.

python/cpython@ee78ba2 ended up with ugly GH line-wrapping of the first line despite the PR python/cpython#2825 title being unwrapped.

Could we remove bpo link in automerge commit? It's somewhat duplicate, also make the backport commit message not neat. It seems currently backport commit message removes the bpo link, any real need to apply two different strategies?

Sometimes I need to manually backport a PR due to API changes. Currently, the only way I know to tell miss-islington not to create a backport PR is removing the "backport to X.Y" label. But I find "backport to X.Y" labels useful since we don't always keep the version field in the bpo issue up-to-date. Can we add a "skip auto backport" label (similar to what "skip news" etc. does)? I'm open to alternative names and as always I can implement it if people find the idea useful.

From https://devguide.python.org/gitbootcamp/#backporting-merged-changes

Example of good backport commit message:

bpo-12345: Improve the spam module (GH-777)

• Add method A to the spam module
• Update the documentation of the spam module

(cherry picked from commit 62adc55)

I agree that the backport PR's number is rarely useful and it would be nice to remove it automatically. Can miss-islington strip the backport PR number in the commit message?

The bot is still broken: python/cpython#12383 (comment) the bot is working for 14 minutes to create a backport, whereas previously it took less than 5 minutes (usually less than 1 minute).

https://discuss.python.org/t/broken-miss-inlington/1030

There's a new Python runtime enviornment in Heroku. You might want to update the bot to use the new environment by editing the runtime.txt file.

See the changelog at https://devcenter.heroku.com/changelog-items/1679
Message:
Python 3.7.4 and 3.6.9 are now available on all stacks. See Heroku Python Support for more details.

The get-pip tool used by the Python Buildpack has also been updated.

(I'm a bot) 🤖

When backporting an issue that has skip-issue and skip-news tags, the backport should get those tags too, otherwise the checks will fail.

If miss-islington failed to backport a PR, she can assign the PR to the core developer who merged it.

I've applied the 🤖 automerge label to an issue whose checks already passed, and was a bit surprised that nothing happened. I had to press Squash and add GH- by hand! (I admit I am very spoiled by useful bots.)

Would it be a good idea automerge when the label is added, if the other conditions are met?

Hum, the bot wrote "@vstinner: Backport status check is done, and it's a success white_check_mark." twice, the second message came 8 minutes after the first one:
python/cpython#7824 (comment)

In backport PRs, if all the required status checks have completed, then go ahead and merge the PR.
Don't need to wait for the unrequired status checks.

Perhaps we can add a line in commit message of who applied the ":robot: automerge" label.
Something like: "approved by: Core Dev"

Since miss-islington uses a GitHub handle in the "Automerge-Triggered-By" 'header', whoever triggered the automerge gets spammed any time a contributor does something strange in their own fork of the repo that causes a new commit with the same "Automerge-Triggered-By" message to be pushed to their fork. For example, I was just notified about this commit this morning.

I would prefer using display name (Zachary Ware, possibly with email address if public) rather than GitHub handle (@zware) for this message, which also makes it easier to tie the committer to the commit since the GitHub handle is not actually present anywhere else in the Git repository.

See if it makes sense to replace all the background tasks from using celery to aiojobs.

https://aiojobs.readthedocs.io/

The downside of auto backport is the slew of several emails per backport. I would like the 'Thanks' comments stopped as they are just noise (to me, at least). Typically, the only person who see it is the core developer who did the merge.

[X.Y] should be in the PR title but not in the backport commit message.

Not sure yet if this is for miss-islington or cherry-picker 😅
Writing it down somewhere before I forgot about the issue.

Perhaps this repo needs @the-knights-who-say-ni but I don't know how to set that up.

For now the automated cherry-pick failed if generated files are changed. These conflicts can be resolved automatically — just regenerate files by running make regen-all (or with more specific targets, depending on affected files).

Now that the cpython 3.6 branch is moving to security-fix-only mode, per our prior practice I have now restricted merges on the 3.6 branch to release managers and administrators. That made me wonder whether that restriction applies to our GitHub bots, particularly the automerge bot. Since I can't easily test this from my GitHub account (as I'm in the release manager team!), does anyone know whether automerges have been being blocked for 3.5 and/or could test for 3.6?

It should assign it back to the approver.

In python/cpython#3548, the pr backports cleanly without conflict, but the test suite failed.

Some ideas of what @miss-islington can do:

• notify the original PR author, or the core dev merging the PR, that the backport failed (leave a comment)
• automatically close the backport PR? because it now requires manual backport. A core dev can close it too.
• re-add the needs backport to [X.Y] label to the original PR

There's a new Python runtime enviornment in Heroku. You might want to update the bot to use the new environment by editing the runtime.txt file.

See the changelog at https://devcenter.heroku.com/changelog-items/1698
Message:
Python version 3.7.5 is now available on all supported Heroku stacks.

In addition, support for new branch Python 3.8.0 was added.

See Python Supported Runtimes for more information.

(I'm a bot) 🤖

I don't know what to do when this bot is broken :-(

I requested a backport from master to 3.7 on python/cpython#13421 The bot wrote "Thanks @erikjanss for the PR, and @vstinner for merging it .. I'm working now to backport this PR to: 3.7." and then nothing, as if it fails but doesn't say anything about its failure.

Who operates this bot? Who knows how to debug it?

I approved the python/cpython#7960 but then blocked the PR by voting -1. And... the bot merged the change anyway!

The bot must take also -1 votes from core developers in account...

Forgive me for barging in here - I have only just discovered this project so I know very little about it, but maybe my tool could help with your automated backports in the cases where cherry-picking causes conflicts due to the commit being backported relying on other commits?

https://github.com/aspiers/git-deps#use-case-1-porting-between-branches

I would certainly not claim that this can always be safely used to fully automate the resolution of conflicts. However it can serve as a useful assistant to a) provide an estimate of the complexity of the porting task and which dependent commits might need to be included in the port.

If I'm off the mark here then feel free to close this, but I thought I'd mention it just in case it's useful :-)

I'm having trouble backporting to 3.7. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.7 label.

Issue seen at: python/cpython#13816 (comment)

There's a new Python runtime enviornment in Heroku. You might want to update the bot to use the new environment by editing the runtime.txt file.

See the changelog at https://devcenter.heroku.com/changelog-items/1699
Message:
Python version 2.7.17 is now available on all supported Heroku stacks.

See Python Supported Runtimes for more information.

(I'm a bot) 🤖