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

Move to GitHub Actions #344

Closed
vstinner opened this issue May 27, 2021 · 24 comments
Closed

Move to GitHub Actions #344

vstinner opened this issue May 27, 2021 · 24 comments

Comments

@vstinner
Copy link
Member

vstinner commented May 27, 2021

Sometimes, the 3 "bedevere/*" bots don't run and it is not possible to merge a PR :-(

miss-islington created python/cpython#26413 backport PR. I approved it. But the following 3 jobs never ran:

  • bedevere/issue-number Expected — Waiting for status to be reported (Required)
  • bedevere/maintenance-branch-pr Expected — Waiting for status to be reported (Required)
  • bedevere/news Expected — Waiting for status to be reported (Required)

Workaround for issue-number and maintenenance branch: edit the PR title, then edit to undo your edit.

Workaround for news: add "skip news" label.

When the PR changes, the bot runs, and it becomes possible to merge the PR.

Who operates these bots? How can I debug these issues?

@Mariatta
Copy link
Member

Who operates these bots?

bedevere is hosted on Heroku. We set up GitHub webhook that send events to the heroku web service.
Sometimes, the webserver was not able to process the webhook event, and therefore it could not post the correct status.

I looked at the webhook event for the PR you mentioned above, and indeed it was getting 500 error. Most of the time, all I have to do is to redeliver the webhook event.

How can I debug these issues?

I think the workaround of closing and re-opening the PR is all you can do.

For me, since I have additional priviliges and access to the Heroku service, I would do these things:

  • go to the repo's webhook settings page, and review the webhook deliveries. Look for errors and attempt to redeliver the webhook
  • go to Heroku and view the logs
  • go to Sentry and see if there is any error alerts

@brettcannon brettcannon changed the title Sometimes, the 3 "bedevere/*" bots don't run Moving to GitHub Actions May 28, 2021
@brettcannon
Copy link
Member

The other option is to move Bedevere to GitHub Actions as the bot is stateless by design. And making it an action means the executions and results will be entirely public.

The reason it has not happened yet is time and deciding how to do it. Since you can register what triggers a workflow in GitHub Actions, it could be a quick-and-dirty lift of checking this repo's code out into a run and then executing it once as if the trigger was a webhook event (the code change on the Bedevere side would be minimal and entirely centralized in https://github.com/python/bedevere/blob/master/bedevere/__main__.py).

The other option is breaking the code up to be a bit better integrated into GitHub Actions so that it can run in parallel, etc. Obviously a bit more work as it would require more code refactoring.

As for time, it's not just making it happen but having the time to sit and watch the initial executions to make sure the workflow file works, no weird failures, etc.

And the news file check can be replaced with https://github.com/brettcannon/check-for-changed-files.

@brettcannon brettcannon changed the title Moving to GitHub Actions Move to GitHub Actions May 28, 2021
@Mariatta
Copy link
Member

Would it be possible to host the actions code not as part of CPython codebase, but separate repo? Cause I think it's "bulky" to include all of bedevere into CPython source, but maybe it's just me.

@brettcannon
Copy link
Member

Yep, you can host it separately (i.e. leave it here). You can use https://github.com/actions/checkout to check out any repo you want, so it could be used to check out Bedevere in a workflow and run the code there since Bedevere doesn't work directly with CPython source but entirely on webhook payloads and stuff you get from the REST API (which I know Mariatta knows, but FYI for everyone else 😄 ).

@nightlark
Copy link

Would you want to have a GitHub Action version of bedevere split up so that there is a separate yaml file for each bedevere action (e.g. issue-number.yaml, news.yaml etc) in the CPython .github/workflows directory?

And a 2nd question if splitting is okay -- would you want to keep most of the logic for the action written in Python (based on the existing bedevere code), or would porting some of the simpler ones to use JavaScript with https://github.com/actions/github-script be okay?

@brettcannon
Copy link
Member

We are definitely not rewriting Bedevere in JavaScript. Replacing some of the checks may be fine with actions from the community if people are okay with that and we take care to not give inappropriate permissions to them (security concern if you give an action that none of us controls write access to the repo). But I do not want our workflow to require someone to know JS in order to be able to understand it or contribute to it.

As for splitting it all up, I had not thought about it specifically. We could reuse the Bedevere code with separate workflows as you suggested so they show up as separate status checks. Or we can keep our lives simple and have a single "Bedevere" workflow that acts somewhat like the bot does now. There's also a middle ground of a "Bedevere" workflow definition that has separate jobs for each of the areas that Bedevere performs work for and then get with an if for each job based on what triggers for it.

@arhadthedev
Copy link
Member

You can use https://github.com/actions/checkout to check out any repo you want

Would you want to have a GitHub Action version of bedevere split up so that there is a separate yaml file for each bedevere action (e.g. issue-number.yaml, news.yaml etc) in the CPython .github/workflows directory?

It would be more idiomatic to wrap Bedevere as a standalone action in its own repository (like actions/checkout does) and call it from CPython workflow files. There is even a template for Python-based actions, so JavaScript is used only for a very thin wrapper to pass action parameters: https://github.com/peter-evans/python-action.

As a result, CPython repository will contain not the whole bot but its minimal invocation, something like this:

# .github/workflows/bedevere.yml
name: Identify missing information

on:
  # synchronize=a push into a PR branch
  pull_request: [opened, edited, synchronize, labeled, unlabeled]
  issue_comment: [created, edited]
  pull_request_review_comment: [created, edited]

jobs:
  check_issue:
    name: Check for bugs.python.org issue numbers and NEWS
    runs-on: ubuntu-latest
    steps:
      - uses: python/bedevere@master
        with:
          token: {{ secrets.SOME_TOKEN_WITH_COMMENT_CREATION_AND_EDITING_RIGHTS }}
          event: {{ github.event }}

@brettcannon
Copy link
Member

We could probably use https://docs.github.com/en/actions/learn-github-actions/reusing-workflows to provide a workflow that can be directly used by the CPython repo.

@arhadthedev
Copy link
Member

Thank you for the link, I did not know about this feature. I agree that switching to a builtin (vendored) action is better because nobody outside CPython can use Bedevere without migration to bugs.python.org for some reason.

@sabderemane
Copy link

Hi folks, @Mariatta talk to me about this issue, I would like to do it 😄
I just want to be sure I understand correctly, should I do all workflows in cpython repository or here in bedevere and it will be reused by others projects from this one ?

@Mariatta
Copy link
Member

Hi @sabderemane, I think it would be better to keep the codebase of bedevere here in this repo, instead of including it in the CPython repo.

@nightlark
Copy link

nightlark commented Nov 16, 2021

It would be more idiomatic to wrap Bedevere as a standalone action in its own repository (like actions/checkout does) and call it from CPython workflow files. There is even a template for Python-based actions, so JavaScript is used only for a very thin wrapper to pass action parameters: https://github.com/peter-evans/python-action.

The JavaScript wrapper shown there isn't needed anymore -- composite run step actions can work for running Python code directly in a GitHub action.

A minimal action.yml would be:

name: 'Hello World'
description: 'Greet someone'
inputs:
outputs:
runs:
  using: "composite"
  steps:
    - run: |
        import sys
        import os
        sys.path.append('${{ github.action_path }}')
        from py_action_import import hello_world
        sys.path.pop()
        hello_world()
      shell: python

Then in the py_action_import.py file in the same repository:

def hello_world():
    print("Hello world!")

I was playing around with this some here: https://github.com/nightlark/test-python-composite-action/tree/main/test-action

@Mariatta
Copy link
Member

I'm not a fan of including so much Python code like the above into the yml file. Seems like an anti-pattern.

@nightlark
Copy link

nightlark commented Nov 16, 2021

I'm not a fan of including so much Python code like the above into the yml file. Seems like an anti-pattern.

That's only 6 lines (!!) of Python code to get it running the bulk of the Bedevere python code, compared to a much larger JavaScript shim as one of the alternatives.

If you think 6 lines is too much, it could easily be brought down to 4 lines (import os was a remnant of other things I was trying and cleaning up sys.path after the import probably isn't needed), or 3 if you make the import start running Bedevere immediately instead of requiring a function call:

import sys
sys.path.append('${{ github.action_path }}')
from py_action_import import hello_world
hello_world()

As an alternative, the default bash shell could be used instead and then it's potentially just a 1 line shell command to run a bedevere.py file:

name: 'Bedevere'
description: 'Run Bedevere'
inputs:
outputs:
runs:
  using: "composite"
  steps:
    - run: python "${{ github.action_path }}/bedevere.py"

(It seems a bit less direct to run a shell command instead of having GitHub Actions just go straight to running a Python interpreter, though I'd guess that any difference in execution time would be unnoticeable, so no real downsides come to mind)

@brettcannon
Copy link
Member

@sabderemane are you still planning to take this on?

@sabderemane
Copy link

sabderemane commented Dec 12, 2021

Yes @brettcannon ! I have already started to do something but I have to review the current codebase to make workflows simples since it will not need a server to run as before. I will ask @Mariatta to have her point of view on a part before I make a draft PR and make the change everywhere.

@brettcannon
Copy link
Member

The steering council just talked about turning on 2FA for the Python org and we realized moving Bedevere to GitHub Actions will need to happen beforehand.

@sabderemane
Copy link

Is there a deadline for the move on 2FA ?
I have asked @Mariatta her point of view on my last changes, I will see in function of her return to move on and make my PR as soon as possible.

@brettcannon
Copy link
Member

No deadline. View my comment more as motivation than anything.

@Mariatta
Copy link
Member

One issue with using Actions is that it require maintainers to "approve" the workflow for contributors before it can be run.
The workflows done by bedevere, like checking for skip issue, skip news, applying "type" labels, should just run by itself without needing maintainer approval.

So I think it would be better to convert bedevere into GitHub App instead of Actions. By converting this to GitHub App, we can resolve the issue where 2FA is needed for the bedevere-bot account.

Note that bedevere-bot's account is also used for the buildbot. I would recommend that buildbot is also updated to either GitHub Actions or GitHub App.

PR for converting bedevere into a GitHub App: #569

PR for converting bedevere into GitHub Actions: #568

@Mariatta
Copy link
Member

To solve the issue where core devs want to be able to re-run workflows/retrigger webhook in case the web app is down, we can grant the "GitHub App manager" access to core devs. Through the GitHub App interface, core devs can view which webhook failed and can re-deliver it.
The GitHub App interface is separate from the CPython repo. Currently bedevere webhook events can only be seen through the CPython repo settings which require admin privileges, and makes it difficult to debug, which I think is one of the main issue in our workflow now.

@pradyunsg
Copy link
Member

We've converted this project to a GitHub App now, which was considered an alternative to converting this to a GitHub Action avoiding the 2FA requirement.

Is moving to GitHub Actions something we still want to do here? One functional argument against it would be that each run of bedevere would require spinning up a whole runner VM, which we have a limited pool size for the python organisation and it would compete with the CI test suite runs for those workers.

@brettcannon
Copy link
Member

If people want to maintain Bedevere as an app then I say close this.

@Mariatta
Copy link
Member

I don't think Actions is appropriate for Bedevere. We need Bedevere to be able to apply/remove labels immediately instead of waiting for someone to approve the workflow.
So I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants