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

Automerge action #10069

Closed
harupy opened this issue Oct 23, 2023 · 12 comments · Fixed by #10210
Closed

Automerge action #10069

harupy opened this issue Oct 23, 2023 · 12 comments · Fixed by #10210
Assignees
Labels
good first issue Good for newcomers has-closing-pr This issue has a closing PR

Comments

@harupy
Copy link
Member

harupy commented Oct 23, 2023

Summary

GitHub's auto-merge feature only checks required jobs. In this repo, we have jobs that are conditionally triggered. Those conditional jobs can't be marked as required (if we marked them as required, they would block the PR forever) but should block the PR. We need our own auto-merge mechanism that has the following features:

  1. Runs every 10 minutes.
  2. Searched for pull requests labeled with automerge
  3. Merge the PR if all the CI checks have passed (both required and non-required ones).

Example code

ChatGPT wrote this code. I'm not sure if this really works. We need to test in a different repository:

name: Automerge PRs

on:
  schedule:
    - cron: '*/10 * * * *'  # Run every 10 minutes

jobs:
  merge:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout code
      uses: actions/checkout@v2

    - name: Automerge PRs with "automerge" label created within the last month
      uses: actions/github-script@v5
      with:
        github-token: ${{secrets.GITHUB_TOKEN}}
        script: |
          const { repo: { owner, repo } } = context;
          
          // Get date from a month ago in ISO format
          const oneMonthAgo = new Date();
          oneMonthAgo.setMonth(oneMonthAgo.getMonth() - 1);
          const sinceDate = oneMonthAgo.toISOString();

          // List PRs with the "automerge" label created within the last month
          const { data: issues } = await github.issues.listForRepo({
            owner,
            repo,
            labels: 'automerge',
            since: sinceDate
          });

          // Filter for pull requests from the list of issues
          const pullRequests = issues.filter(issue => issue.pull_request);

          for (const pr of pullRequests) {
            // Fetch PR details
            const { data: pullRequest } = await github.pulls.get({
              owner,
              repo,
              pull_number: pr.number
            });

            // Check the status of all checks for the PR
            const { data: checkRuns } = await github.checks.listForRef({
              owner,
              repo,
              ref: pullRequest.head.sha
            });

            const allChecksPassed = checkRuns.check_runs.every(run => run.conclusion === 'success');

            if (allChecksPassed) {
              await github.pulls.merge({
                owner,
                repo,
                pull_number: pr.number,
                commit_title: `Merge pull request #${pr.number}`,
                commit_message: `automatic merge of PR #${pr.number}`
              });
              console.log(`Merged PR #${pr.number}`);
            } else {
              console.log(`Checks not ready for PR #${pr.number}. Skipping merge.`);
            }
          }

Notes

  • Make sure to open a PR from a non-master branch.

  • Sign off the commit using the -s flag when making a commit:

    git commit -s -m "..."
             # ^^ make sure to use this
  • Include #{issue_number} (e.g. #123) in the PR description when opening a PR.

@harupy harupy added the good first issue Good for newcomers label Oct 23, 2023
@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

More robust code:

name: Automerge PRs

on:
  schedule:
    - cron: '*/10 * * * *'  # Run every 10 minutes

jobs:
  merge:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout code
      uses: actions/checkout@v2

    - name: Automerge PRs with "automerge" label created within the last month and are mergeable
      uses: actions/github-script@v5
      with:
        github-token: ${{secrets.GITHUB_TOKEN}}
        script: |
          const { repo: { owner, repo } } = context;

          const MAX_RETRIES = 3;
          const RETRY_INTERVAL_MS = 10000;  // 10 seconds

          // Get date from a month ago in ISO format
          const oneMonthAgo = new Date();
          oneMonthAgo.setMonth(oneMonthAgo.getMonth() - 1);
          const sinceDate = oneMonthAgo.toISOString();

          // List PRs with the "automerge" label created within the last month
          const { data: issues } = await github.issues.listForRepo({
            owner,
            repo,
            labels: 'automerge',
            since: sinceDate
          });

          // Filter for pull requests from the list of issues
          const pullRequests = issues.filter(issue => issue.pull_request);

          for (const pr of pullRequests) {
            let pullRequest;

            for (let i = 0; i < MAX_RETRIES; i++) {
              // Fetch PR details
              pullRequest = await github.pulls.get({
                owner,
                repo,
                pull_number: pr.number
              }).then(res => res.data);

              if (pullRequest.mergeable !== null) {
                break;
              }

              console.log(`Waiting for mergeability calculation for PR #${pr.number}...`);
              await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL_MS));
            }

            // If mergeable is false or still null after retries, skip this PR
            if (pullRequest.mergeable === null || pullRequest.mergeable === false) {
              console.log(`PR #${pr.number} is not mergeable. Skipping this PR.`);
              continue;
            }

            // Check the status of all checks for the PR
            const { data: checkRuns } = await github.checks.listForRef({
              owner,
              repo,
              ref: pullRequest.head.sha
            });

            const allChecksPassed = checkRuns.check_runs.every(run => run.conclusion === 'success');

            if (allChecksPassed) {
              try {
                await github.pulls.merge({
                  owner,
                  repo,
                  pull_number: pr.number,
                  commit_title: `Merge pull request #${pr.number}`,
                  commit_message: `automatic merge of PR #${pr.number}`
                });
                console.log(`Merged PR #${pr.number}`);
              } catch (error) {
                console.log(`Failed to merge PR #${pr.number}. Reason: ${error.message}`);
              }
            } else {
              console.log(`Checks not ready or PR not approved for PR #${pr.number}. Skipping merge.`);
            }
          }

@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

@TomeHirata Would you be interested in working on this?

@TomeHirata
Copy link
Contributor

TomeHirata commented Oct 23, 2023

@harupy Yes, please let me work on this. I have several questions I'd like to clarify

  1. What is the condition when automerge label is attached? Is it a manual process?
  2. Can I understand that we will attach the label to PR rather than issue? I asked since the pseudo-code tries to filter issues instead of PRs
  3. Can't we have a job that is triggered when the label is attached is added instead of having a scheduled job? It may be able to save the computational resources (ref)

@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

What is the condition when automerge label is attached? Is it a manual process?

Yes, it's manual. If your PR is approved and all you need to do is wait, then you can apply this label.

Can I understand that we will attach the label to PR rather than issue? I asked since the pseudo-code tries to filter issues instead of PRs

const pullRequests = issues.filter(issue => issue.pull_request); filters out issues.

Can't we have a job that is triggered when the label is attached instead of having a scheduled job? It may be able to save the computational resources (ref)

Good question.

  • You might still have running jobs when you apply the label.
  • What if you push a commit after you apply the label?

We can increase the interval (e.g. 30 minutes or 1 hour) if necessary.

@TomeHirata
Copy link
Contributor

TomeHirata commented Oct 23, 2023

I understand, thank you for the clarification. I also learned that github.issues API can also get pull requests (ref).
Let me work on this task.

@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

@TomeHirata Thanks! Can you create a repo (that's okay to be broken) for testing?

@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

We can create the action above + one more job that protects master/main branch, and then file a PR with automerge label applied.

@harupy
Copy link
Member Author

harupy commented Oct 23, 2023

Asked ChatGPT to make a few updates:

name: Automerge PRs

on:
  schedule:
    - cron: '*/10 * * * *'  # Run every 10 minutes

jobs:
  merge:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout code
      uses: actions/checkout@v2

    - name: Automerge PRs with "automerge" label created within the last month and are mergeable
      uses: actions/github-script@v5
      with:
        github-token: ${{secrets.GITHUB_TOKEN}}
        script: |
          const { repo: { owner, repo } } = context;

          const MAX_RETRIES = 3;
          const RETRY_INTERVAL_MS = 10000;  // 10 seconds
          const MERGE_INTERVAL_MS = 5000;   // 5 seconds pause after a merge

          async function sleep(ms) {
            return new Promise(resolve => setTimeout(resolve, ms));
          }

          async function logRateLimit() {
            const { data: rateLimit } = await github.rateLimit.get();
            console.log(`Rate limit remaining: ${rateLimit.rate.remaining}`);
            console.log(`Rate limit resets at: ${new Date(rateLimit.rate.reset * 1000).toISOString()}`);
          }

          async function fetchPullRequestDetails(prNumber) {
            for (let i = 0; i < MAX_RETRIES; i++) {
              const pullRequest = await github.pulls.get({
                owner,
                repo,
                pull_number: prNumber
              }).then(res => res.data);

              if (pullRequest.mergeable !== null) {
                return pullRequest;
              }

              console.log(`Waiting for mergeability calculation for PR #${prNumber}...`);
              await sleep(RETRY_INTERVAL_MS);
            }
            return null;
          }

          async function isPRApproved(prNumber) {
            const { data: reviews } = await github.pulls.listReviews({
              owner,
              repo,
              pull_number: prNumber
            });
            return reviews.some(review => review.state === 'APPROVED');
          }

          async function areAllChecksPassed(sha) {
            const { data: checkRuns } = await github.checks.listForRef({
              owner,
              repo,
              ref: sha
            });
            return checkRuns.check_runs.every(run => run.conclusion === 'success');
          }

          // Get date from a month ago in ISO format
          const oneMonthAgo = new Date();
          oneMonthAgo.setMonth(oneMonthAgo.getMonth() - 1);
          const sinceDate = oneMonthAgo.toISOString();

          // List PRs with the "automerge" label created within the last month
          const { data: issues } = await github.issues.listForRepo({
            owner,
            repo,
            labels: 'automerge',
            since: sinceDate
          });

          // Filter for pull requests from the list of issues
          const pullRequests = issues.filter(issue => issue.pull_request);

          for (const pr of pullRequests) {
            const pullRequest = await fetchPullRequestDetails(pr.number);

            if (!pullRequest || pullRequest.mergeable !== true) {
              console.log(`PR #${pr.number} is not mergeable or could not fetch details. Skipping this PR.`);
              await logRateLimit();
              continue;
            }

            if (!await isPRApproved(pr.number)) {
              console.log(`PR #${pr.number} hasn't been approved. Skipping merge.`);
              await logRateLimit();
              continue;
            }

            if (await areAllChecksPassed(pullRequest.head.sha)) {
              try {
                await github.pulls.merge({
                  owner,
                  repo,
                  pull_number: pr.number
                });
                console.log(`Merged PR #${pr.number}`);

                await sleep(MERGE_INTERVAL_MS);
                await logRateLimit();
              } catch (error) {
                console.log(`Failed to merge PR #${pr.number}. Reason: ${error.message}`);
              }
            } else {
              console.log(`Checks not ready for PR #${pr.number}. Skipping merge.`);
              await logRateLimit();
            }
          }

@TomeHirata
Copy link
Contributor

TomeHirata commented Oct 24, 2023

@harupy

Can you create a repo (that's okay to be broken) for testing?

Sure, will create a small test repo. Could you assign this ticket to me so that it's easy to track?
Also, it looks like that github-action supports a retry functionality, so we can simplify the code by utilizing it. Let me try it and ask for a review.
https://github.com/actions/github-script#retries

@harupy
Copy link
Member Author

harupy commented Oct 24, 2023

@TomeHirata Assigned!

@harupy
Copy link
Member Author

harupy commented Oct 24, 2023

a retry functionality, so we can simplify the code by utilizing it

I didn't know actions/github-script supports this. We can use it if necessary.

@github-actions
Copy link

@mlflow/mlflow-team Please assign a maintainer and start triaging this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers has-closing-pr This issue has a closing PR
Projects
None yet
2 participants