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.

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

🚸 🐍 πŸ’ ⛏ Integrate cherry picker #41403

Merged
merged 3 commits into from
Jul 12, 2018

Conversation

webknjaz
Copy link
Member

SUMMARY

This adds config for using cherry-picker tool from Python Core Workflow. No docs yet, but it should be enough for people to start trying it out.

ISSUE TYPE
  • Feature Pull Request
  • Docs Pull Request
  • New Tool Config Pull Request
COMPONENT NAME

β€’

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

β€’

@webknjaz webknjaz self-assigned this Jun 11, 2018
@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 11, 2018
@abadger
Copy link
Contributor

abadger commented Jun 11, 2018

Documentation is important. A few examples on how to create a backport, at least. I have no problem with this PR, otherwise.

@sivel
Copy link
Member

sivel commented Jun 11, 2018

Documentation is important. A few examples on how to create a backport, at least. I have no problem with this PR, otherwise.

Agreed. I think specifically showing an example that roughly duplicates our manual process using cherry-picker would be extremely helpful.

@webknjaz
Copy link
Member Author

Yeah, I was thinking about it. It was just an end of the day and I postponed it :)

Will add it.

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 12, 2018
@webknjaz webknjaz removed the needs_triage Needs a first human triage before being processed. label Jun 12, 2018
This enables developers to use cherry-picker for backporting purposes.
This tool originally comes from Core Python Development Workflow.

Ref: https://pypi.org/p/cherry-picker
Ref: https://github.com/python/core-workflow/tree/master/cherry_picker
@webknjaz webknjaz force-pushed the feature/integrate-cherry-picker branch from 8de686f to 437f296 Compare June 12, 2018 15:27
@webknjaz webknjaz changed the title Feature/integrate cherry picker Integrate cherry picker Jun 12, 2018
@webknjaz webknjaz requested a review from acozine June 12, 2018 15:37
@@ -82,6 +82,48 @@ pull request to backport the change to a previous stable branch.
but it can be helpful, especially when making multiple backport PRs for
multiple stable branches.

.. note::

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

@webknjaz
Copy link
Member Author

@acozine could you please review the doc change here?

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 12, 2018
@abadger
Copy link
Contributor

abadger commented Jun 12, 2018

@webknjaz I tried to use cherry_picker for a PR today and had no success. The examples in the docs probably need a little work:

  • For the example, you need to be in a clone where origin == ansible/ansible. Many of us use origin == my_fork so the example should make clear that this needs to be done in a clone of git@github.com:ansible/ansible (or, if cherry_picker is flexible enough, an example where origin == my_fork)

  • I tried several things for values of fork in --pr-remote fork. Should document that you need to git remote add my_fork git@github.com:username/ansible and then use --pr-remote my_fork.

  • I still didn't get it to work after that. Here's my command line and the problems I had:

$ cherry_picker --pr-remote abadger 336b3762b23a64e355cfa3efba11ddf5bdd7f0d8 stable-2.6 stable-2.5 stable-2.4
🐍 πŸ’ ⛏

Now backporting '336b3762b23a64e355cfa3efba11ddf5bdd7f0d8' into 'stable-2.6'
Switched to a new branch 'backport-336b376-stable-2.6'
Branch backport-336b376-stable-2.6 set up to track remote branch stable-2.6 from origin.

[backport-336b376-stable-2.6 eaa83dba70] no_log even when task_result doesn't provide key
 Author: Brian Coca <brian.coca+git@gmail.com>
 Date: Thu Jun 7 17:38:20 2018 -0400
 2 files changed, 28 insertions(+), 1 deletion(-)

Failed to push to abadger ☹
Switched to branch 'devel'
Your branch is up-to-date with 'origin/devel'.

Deleted branch backport-336b376-stable-2.6 (was b5ee4a44a3).

branch backport-336b376-stable-2.6 has been deleted.
Now backporting '336b3762b23a64e355cfa3efba11ddf5bdd7f0d8' into 'stable-2.5'
Switched to a new branch 'backport-336b376-stable-2.5'
Branch backport-336b376-stable-2.5 set up to track remote branch stable-2.5 from origin.

[backport-336b376-stable-2.5 f5c404590f] no_log even when task_result doesn't provide key
 Author: Brian Coca <brian.coca+git@gmail.com>
 Date: Thu Jun 7 17:38:20 2018 -0400
 2 files changed, 28 insertions(+), 1 deletion(-)

To github.com:abadger/ansible
 * [new branch]            backport-336b376-stable-2.5 -> stable-2.5

Backport PR URL:
https://github.com/ansible/ansible/compare/stable-2.5...abadger:backport-336b376-stable-2.5?expand=1
Switched to branch 'devel'
Your branch is up-to-date with 'origin/devel'.

Deleted branch backport-336b376-stable-2.5 (was e080151577).

branch backport-336b376-stable-2.5 has been deleted.
Now backporting '336b3762b23a64e355cfa3efba11ddf5bdd7f0d8' into 'stable-2.4'
Switched to a new branch 'backport-336b376-stable-2.4'
Branch backport-336b376-stable-2.4 set up to track remote branch stable-2.4 from origin.

[backport-336b376-stable-2.4 5a22f21f4f] no_log even when task_result doesn't provide key
 Author: Brian Coca <brian.coca+git@gmail.com>
 Date: Thu Jun 7 17:38:20 2018 -0400
 2 files changed, 28 insertions(+), 1 deletion(-)

To github.com:abadger/ansible
 * [new branch]            backport-336b376-stable-2.4 -> stable-2.4

Backport PR URL:
https://github.com/ansible/ansible/compare/stable-2.4...abadger:backport-336b376-stable-2.4?expand=1
Switched to branch 'devel'
Your branch is up-to-date with 'origin/devel'.

Deleted branch backport-336b376-stable-2.4 (was 35cff6c899).

branch backport-336b376-stable-2.4 has been deleted.

The stable-2.6 branch failed. There's not enough information given for me to tell why.

The stbale-2.4 and stable-2.5 branches appear to have worked but the browser windows that cherry_picker opened up show that there's no changes to commit: https://github.com/ansible/ansible/compare/stable-2.5...abadger:backport-336b376-stable-2.5?expand=1
on closer examination, the branches do not appear to exist in the abadger/ansible repo: https://github.com/abadger/ansible/branches/all?utf8=%E2%9C%93&query=backport

Running cherry_picker on just the stable-2.5 branch, I get the failure to push message so my guess is that all three failed to push but only the stable-2.6 branch realized that it had an error. The other two weren't seen as an error because stable-2.6 had already failed.

@webknjaz
Copy link
Member Author

Thanks for the feedback @abadger :) I'll look into it deeper.

if cherry_picker is flexible enough, an example where origin == my_fork

I think @Mariatta will be glad to accept another patch for such flexibility.

You probably want --pr-remote origin and cherry-picker uses upstream with fallback to origin for upstream branches

backport-336b376-stable-2.5 -> stable-2.5

This is weired, I'd expect backport-336b376-stable-2.5 -> backport-336b376-stable-2.5 here.

@webknjaz
Copy link
Member Author

It looks like 17830a0 was created.
@abadger do you have any git customizations in your setup?

@Mariatta
Copy link

you need to be in a clone where origin == ansible/ansible. Many of us use origin == my_fork

The default behavior is:

origin = myfork
upstream = python/cpython or (aio-libs/aiohttp or ansible/ansible, configurable in .cherry_picker.toml)

Sounds to me that it is already working as expected?

I haven't look at the rest of the issues mentioned here, will find time later this week.

@Mariatta
Copy link

cherry_picker --pr-remote abadger 336b3762b23a64e355cfa3efba11ddf5bdd7f0d8 stable-2.6 stable-2.5 stable-2.4

Seems like 336b376 was already cherry-picked into stable-2.4 , so there is nothing to backport.

@webknjaz
Copy link
Member Author

@abadger such change makes sense to me.

I suspect that you might have (1) outdated version of Git and/or (2) custom global user config (please post your cat ~/.gitconfig).

Have you tried running this on the different machine?

P.S. My Git is:

$ git --version
git version 2.17.1

@webknjaz
Copy link
Member Author

@abadger by the way, if it makes sense for you, you can try exporting GH_AUTH with your github access token and it will create a PR for you automatically without having you to go through browser.

@webknjaz
Copy link
Member Author

I'm not sure, but you might have a custom push.default configured: http://www.fleekitsolutions.com/git/difference-between-push-default-matching-simple

@abadger
Copy link
Contributor

abadger commented Jun 15, 2018

Hmmm... I thought I posted my .gitconfig earlier but I must have started to edit out the auth tokens and then walked away and forgot. I do seem to have [push] default set. No idea why.

[giggle]
        compact-mode = true
        main-window-maximized = false
        main-window-geometry = 993x665+0+35
        main-window-view = HistoryView
[user]
        name = Toshio Kuratomi
        signingkey = CD84EE48

[diff]
    color = auto
    rename = copy

[pager]
    color = true
[status]
    color = auto
[push]
        default = current
        default = tracking
[merge]
        tool = meld
[alias]
        diff = diff --src-prefix='a/ ' --dst-prefix='b/ '
        pr = 'pull --rebase'
#       log = log --follow

[core]
        editor = gvim -v
        pager = less

@abadger
Copy link
Contributor

abadger commented Jun 16, 2018

Okay, PR's submitted upstream for the bugs I've run into: python/core-workflow#265 and python/core-workflow#264

@Mariatta Is there a reason you don't want to let "upstream" be settable? I've got a half-done change to add --upstream-repo which is just like --pr-repo which I'll finish off, document, and submit if you want it. But if you have strong feelings that you don't want it, I'll just keep that as a local change.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 22, 2018
Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

@webknjaz I meant to make my comments a review - take a look and let me know what you think

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 28, 2018
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 12, 2018
@webknjaz webknjaz changed the title Integrate cherry picker 🚸 Integrate cherry picker Jul 12, 2018
@webknjaz webknjaz changed the title 🚸 Integrate cherry picker 🚸 🐍 πŸ’ ⛏ Integrate cherry picker Jul 12, 2018
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 12, 2018
@webknjaz
Copy link
Member Author

Failures are completely unrelated:

2018-07-12 14:53:13 =================================== FAILURES ===================================
2018-07-12 14:53:13 _______________________________ test_upload_api ________________________________
2018-07-12 14:53:13 monkeypatch = <_pytest.monkeypatch.MonkeyPatch instance at 0x7605710>
2018-07-12 14:53:13 
2018-07-12 14:53:13     def test_upload_api(monkeypatch):
2018-07-12 14:53:13         class FakeConnection:
2018-07-12 14:53:13     
2018-07-12 14:53:13             def put_rest_api(self, *args, **kwargs):
2018-07-12 14:53:13                 assert kwargs["body"] == "the-swagger-text-is-fake"
2018-07-12 14:53:13                 return {"msg": "success!"}
2018-07-12 14:53:13     
2018-07-12 14:53:13         def return_fake_connection(*args, **kwargs):
2018-07-12 14:53:13             return FakeConnection()
2018-07-12 14:53:13     
2018-07-12 14:53:13 >       monkeypatch.setattr(agw, "boto3_conn", return_fake_connection)
2018-07-12 14:53:13 E       AttributeError: <module 'ansible.modules.cloud.amazon.aws_api_gateway' from '/root/ansible/lib/ansible/modules/cloud/amazon/aws_api_gateway.py'> has no attribute 'boto3_conn'
2018-07-12 14:53:13 
2018-07-12 14:53:13 test/units/modules/cloud/amazon/test_api_gateway.py:56: AttributeError
2018-07-12 14:53:13 ============= 1 failed, 3665 passed, 155 skipped in 593.00 seconds =============

Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

I'm happy with the docs side of this PR, thanks @webknjaz

@webknjaz
Copy link
Member Author

Thanks, @acozine! I'm merging this then. We can improve things in follow-up PRs. I'm going to ignore CI failures since those are unrelated.

@webknjaz webknjaz merged commit 97cc0cc into ansible:devel Jul 12, 2018
mattclay pushed a commit to mattclay/ansible that referenced this pull request Jan 10, 2019
This enables developers to use cherry-picker for backporting purposes.
This tool originally comes from Core Python Development Workflow.

Ref: https://pypi.org/p/cherry-picker
Ref: https://github.com/python/core-workflow/tree/master/cherry_picker

Also:
* πŸ“ Add docs about supporting cherry-picker
(cherry picked from commit 97cc0cc)

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
mattclay pushed a commit to mattclay/ansible that referenced this pull request Jan 10, 2019
This enables developers to use cherry-picker for backporting purposes.
This tool originally comes from Core Python Development Workflow.

Ref: https://pypi.org/p/cherry-picker
Ref: https://github.com/python/core-workflow/tree/master/cherry_picker

Also:
* πŸ“ Add docs about supporting cherry-picker
(cherry picked from commit 97cc0cc)

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
mattclay pushed a commit that referenced this pull request Jan 10, 2019
This enables developers to use cherry-picker for backporting purposes.
This tool originally comes from Core Python Development Workflow.

Ref: https://pypi.org/p/cherry-picker
Ref: https://github.com/python/core-workflow/tree/master/cherry_picker

Also:
* πŸ“ Add docs about supporting cherry-picker
(cherry picked from commit 97cc0cc)

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
mattclay pushed a commit that referenced this pull request Jan 10, 2019
This enables developers to use cherry-picker for backporting purposes.
This tool originally comes from Core Python Development Workflow.

Ref: https://pypi.org/p/cherry-picker
Ref: https://github.com/python/core-workflow/tree/master/cherry_picker

Also:
* πŸ“ Add docs about supporting cherry-picker
(cherry picked from commit 97cc0cc)

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
@szb512
Copy link

szb512 commented Mar 22, 2019

We could let python-pip install it.

@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants