From 930bc781d5c2a3ba40f2cab9d4b6fc74a20ed217 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 28 Feb 2022 13:57:37 +0100 Subject: [PATCH 01/14] feat: add auto-merge feature for gitlab --- bin/cml/pr.js | 5 +++++ src/cml.js | 6 ++++-- src/drivers/gitlab.js | 26 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/bin/cml/pr.js b/bin/cml/pr.js index 9eab83b12..522faf540 100755 --- a/bin/cml/pr.js +++ b/bin/cml/pr.js @@ -19,6 +19,11 @@ exports.builder = (yargs) => type: 'boolean', description: 'Output in markdown format [](url).' }, + autoMerge: { + type: 'boolean', + description: + 'If enabled, PR/MR will be marked for automatic merging (only works on GitLab).' + }, remote: { type: 'string', default: GIT_REMOTE, diff --git a/src/cml.js b/src/cml.js index b62d28ee0..b32716208 100755 --- a/src/cml.js +++ b/src/cml.js @@ -340,7 +340,8 @@ class CML { const { remote = GIT_REMOTE, globs = ['dvc.lock', '.gitignore'], - md + md, + autoMerge } = opts; await this.ci(opts); @@ -405,7 +406,8 @@ Automated commits for ${this.repo}/commit/${sha} created by CML. source, target, title, - description + description, + autoMerge }); return renderPr(url); diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index 41043792c..6df1bc38c 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -235,7 +235,7 @@ class Gitlab { async prCreate(opts = {}) { const projectPath = await this.projectPath(); - const { source, target, title, description } = opts; + const { source, target, title, description, autoMerge } = opts; const endpoint = `/projects/${projectPath}/merge_requests`; const body = new URLSearchParams(); @@ -244,15 +244,37 @@ class Gitlab { body.append('title', title); body.append('description', description); - const { web_url: url } = await this.request({ + const { web_url: url, iid } = await this.request({ endpoint, method: 'POST', body }); + if (autoMerge) { + await this.prAutoMerge({ mergeRequestId: iid }); + } + return url; } + /** + * @param {{ mergeRequestId: string }} param0 + * @returns {Promise} + */ + async prAutoMerge({ mergeRequestId }) { + const projectPath = await this.projectPath(); + + const endpoint = `/projects/${projectPath}/merge_requests/${mergeRequestId}/merge`; + const body = new URLSearchParams(); + body.append('merge_when_pipeline_succeeds', true); + + await this.request({ + endpoint, + method: 'POST', + body + }); + } + async prCommentCreate(opts = {}) { const projectPath = await this.projectPath(); const { report, prNumber } = opts; From e4c8b6f04dcccc4d114b4fea2e140647ba676b2b Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 1 Mar 2022 08:58:39 +0100 Subject: [PATCH 02/14] use put request --- src/drivers/gitlab.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index 6df1bc38c..660f2ef6d 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -270,7 +270,7 @@ class Gitlab { await this.request({ endpoint, - method: 'POST', + method: 'PUT', body }); } From dc91ba96088ec5fef97bd5677af12274d9c943b6 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 1 Mar 2022 11:08:32 +0100 Subject: [PATCH 03/14] give API some time to settle --- src/drivers/gitlab.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index 660f2ef6d..bf498611f 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -251,6 +251,7 @@ class Gitlab { }); if (autoMerge) { + await new Promise((resolve) => setTimeout(resolve, 20000)); await this.prAutoMerge({ mergeRequestId: iid }); } From 45491b78832713beef4e0f3ffd33c3a30f84f183 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 16:41:00 +0100 Subject: [PATCH 04/14] feat: use exponential-backoff --- package-lock.json | 11 +++++++++++ package.json | 1 + src/drivers/gitlab.js | 14 ++++++++------ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index ff94ba0bf..db9dfd631 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "@octokit/rest": "18.0.0", "colors": "1.4.0", "ec2-spot-notification": "^2.0.3", + "exponential-backoff": "^3.1.0", "form-data": "^3.0.1", "fs-extra": "^9.1.0", "git-url-parse": "^11.6.0", @@ -3311,6 +3312,11 @@ "node": "^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0" } }, + "node_modules/exponential-backoff": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/exponential-backoff/-/exponential-backoff-3.1.0.tgz", + "integrity": "sha512-oBuz5SYz5zzyuHINoe9ooePwSu0xApKWgeNzok4hZ5YKXFh9zrQBEM15CXqoZkJJPuI2ArvqjPQd8UKJA753XA==" + }, "node_modules/extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", @@ -10595,6 +10601,11 @@ "jest-message-util": "^27.5.1" } }, + "exponential-backoff": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/exponential-backoff/-/exponential-backoff-3.1.0.tgz", + "integrity": "sha512-oBuz5SYz5zzyuHINoe9ooePwSu0xApKWgeNzok4hZ5YKXFh9zrQBEM15CXqoZkJJPuI2ArvqjPQd8UKJA753XA==" + }, "extend": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/extend/-/extend-3.0.2.tgz", diff --git a/package.json b/package.json index f950073c4..bf1a43f3c 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "@octokit/rest": "18.0.0", "colors": "1.4.0", "ec2-spot-notification": "^2.0.3", + "exponential-backoff": "^3.1.0", "form-data": "^3.0.1", "fs-extra": "^9.1.0", "git-url-parse": "^11.6.0", diff --git a/src/drivers/gitlab.js b/src/drivers/gitlab.js index bf498611f..121740b3a 100644 --- a/src/drivers/gitlab.js +++ b/src/drivers/gitlab.js @@ -6,6 +6,7 @@ const fs = require('fs').promises; const fse = require('fs-extra'); const { resolve } = require('path'); const ProxyAgent = require('proxy-agent'); +const { backOff } = require('exponential-backoff'); const { fetchUploadData, download, exec } = require('../utils'); @@ -251,7 +252,6 @@ class Gitlab { }); if (autoMerge) { - await new Promise((resolve) => setTimeout(resolve, 20000)); await this.prAutoMerge({ mergeRequestId: iid }); } @@ -269,11 +269,13 @@ class Gitlab { const body = new URLSearchParams(); body.append('merge_when_pipeline_succeeds', true); - await this.request({ - endpoint, - method: 'PUT', - body - }); + await backOff(() => + this.request({ + endpoint, + method: 'PUT', + body + }) + ); } async prCommentCreate(opts = {}) { From de85dd75bf3eb2fa060ab99a705edd2f129dfafa Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 17:06:22 +0100 Subject: [PATCH 05/14] feat: add github integration --- package-lock.json | 1 + package.json | 1 + src/drivers/github.js | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index db9dfd631..c80d5b40d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "@actions/github": "^4.0.0", "@npcz/magic": "^1.3.12", "@octokit/core": "^3.5.1", + "@octokit/graphql": "^4.8.0", "@octokit/plugin-throttling": "^3.5.2", "@octokit/rest": "18.0.0", "colors": "1.4.0", diff --git a/package.json b/package.json index bf1a43f3c..b3869b71c 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "@actions/github": "^4.0.0", "@npcz/magic": "^1.3.12", "@octokit/core": "^3.5.1", + "@octokit/graphql": "^4.8.0", "@octokit/plugin-throttling": "^3.5.2", "@octokit/rest": "18.0.0", "colors": "1.4.0", diff --git a/src/drivers/github.js b/src/drivers/github.js index 04272adf0..93ace9b58 100644 --- a/src/drivers/github.js +++ b/src/drivers/github.js @@ -6,6 +6,7 @@ const fetch = require('node-fetch'); const github = require('@actions/github'); const { Octokit } = require('@octokit/rest'); +const { withCustomRequest } = require('@octokit/graphql'); const { throttling } = require('@octokit/plugin-throttling'); const tar = require('tar'); const ProxyAgent = require('proxy-agent'); @@ -313,12 +314,18 @@ class Github { } async prCreate(opts = {}) { - const { source: head, target: base, title, description: body } = opts; + const { + source: head, + target: base, + title, + description: body, + autoMerge + } = opts; const { owner, repo } = ownerRepo({ uri: this.repo }); const { pulls } = octokit(this.token, this.repo); const { - data: { html_url: htmlUrl } + data: { html_url: htmlUrl, node_id: nodeId } } = await pulls.create({ owner, repo, @@ -328,9 +335,35 @@ class Github { body }); + if (autoMerge) { + await this.prAutoMerge({ pullRequestId: nodeId }); + } + return htmlUrl; } + /** + * @param {{ pullRequestId: number }} param0 + * @returns {Promise} + */ + async prAutoMerge({ pullRequestId }) { + const octo = octokit(this.token, this.repo); + const graphql = withCustomRequest(octo.request); + + await graphql( + ` + mutation { + enablePullRequestAutoMerge(input: { pullRequestId: $pullRequestId }) { + clientMutationId + } + } + `, + { + pullRequestId + } + ); + } + async prCommentCreate(opts = {}) { const { report: body, prNumber } = opts; const { owner, repo } = ownerRepo({ uri: this.repo }); From d60bb438aab52e98e58a9d3da58862a622c3d1c2 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 17:11:10 +0100 Subject: [PATCH 06/14] feat: add error to bitbucket --- src/drivers/bitbucket_cloud.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/drivers/bitbucket_cloud.js b/src/drivers/bitbucket_cloud.js index 6f551b573..f123c729d 100644 --- a/src/drivers/bitbucket_cloud.js +++ b/src/drivers/bitbucket_cloud.js @@ -108,7 +108,13 @@ class BitbucketCloud { async prCreate(opts = {}) { const { projectPath } = this; - const { source, target, title, description } = opts; + const { source, target, title, description, autoMerge } = opts; + + if (autoMerge) { + throw new Error( + "Bitbucket Cloud doesn't allow Auto-Merging via API. Interested in this feature? Leave a thumbs-up here: TODO ADD A LINK" + ); + } const body = JSON.stringify({ title, From 4bdd2cccde6e74f77ab3310fc231e30009c0e260 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 17:11:38 +0100 Subject: [PATCH 07/14] chore: update comment --- bin/cml/pr.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/cml/pr.js b/bin/cml/pr.js index 522faf540..2e5b731cc 100755 --- a/bin/cml/pr.js +++ b/bin/cml/pr.js @@ -22,7 +22,7 @@ exports.builder = (yargs) => autoMerge: { type: 'boolean', description: - 'If enabled, PR/MR will be marked for automatic merging (only works on GitLab).' + 'If enabled, PR/MR will be marked for automatic merging (doesnt work on BitBucket).' }, remote: { type: 'string', From d9bf091c2221326d7a7d78a095f98ce15a5d69d0 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 18:04:58 +0100 Subject: [PATCH 08/14] feat: add some nice error messages --- src/drivers/github.js | 70 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/src/drivers/github.js b/src/drivers/github.js index 93ace9b58..9b6ab24b6 100644 --- a/src/drivers/github.js +++ b/src/drivers/github.js @@ -336,32 +336,78 @@ class Github { }); if (autoMerge) { - await this.prAutoMerge({ pullRequestId: nodeId }); + await this.prAutoMerge({ pullRequestId: nodeId, base }); } return htmlUrl; } /** - * @param {{ pullRequestId: number }} param0 + * @param {string} branch + * @returns {Promise} + */ + async isProtected(branch) { + const octo = octokit(this.token, this.repo); + const { owner, repo } = this.ownerRepo(); + try { + await octo.repos.getBranchProtection({ + branch, + owner, + repo + }); + return true; + } catch (error) { + if (error.message === 'Branch not protected') { + return false; + } + throw error; + } + } + + /** + * @param {{ pullRequestId: number, base: string }} param0 * @returns {Promise} */ - async prAutoMerge({ pullRequestId }) { + async prAutoMerge({ pullRequestId, base }) { const octo = octokit(this.token, this.repo); const graphql = withCustomRequest(octo.request); - await graphql( - ` - mutation { - enablePullRequestAutoMerge(input: { pullRequestId: $pullRequestId }) { - clientMutationId + try { + await graphql( + ` + mutation autoMerge($pullRequestId: ID!) { + enablePullRequestAutoMerge( + input: { pullRequestId: $pullRequestId } + ) { + clientMutationId + } } + `, + { + pullRequestId + } + ); + } catch (error) { + if ( + error.message.includes("Can't enable auto-merge for this pull request") + ) { + const { owner, repo } = this.ownerRepo(); + const settingsUrl = `https://github.com/${owner}/${repo}/settings`; + + const isProtected = await this.isProtected(base); + if (!isProtected) { + throw new Error( + `Enabling Auto-Merge failed. Please set up branch protection and add "required status checks" for branch '${base}': ${settingsUrl}/branches` + ); } - `, - { - pullRequestId + + throw new Error( + `Enabling Auto-Merge failed. Enable the feature in your repository settings: ${settingsUrl}#merge_types_auto_merge` + ); } - ); + + throw error; + } } async prCommentCreate(opts = {}) { From 7ea7d6b256e4f5db6a18905ddaf54d2c2a436c06 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 9 Mar 2022 18:05:26 +0100 Subject: [PATCH 09/14] feat: dont put "[skip ci]" for autoMerge usecases --- src/cml.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cml.js b/src/cml.js index b32716208..013361348 100755 --- a/src/cml.js +++ b/src/cml.js @@ -393,7 +393,11 @@ class CML { await exec(`git checkout -B ${target} ${sha}`); await exec(`git checkout -b ${source}`); await exec(`git add ${paths.join(' ')}`); - await exec(`git commit -m "CML PR for ${shaShort} [skip ci]"`); + let commitMessage = `CML PR for ${shaShort}`; + if (!autoMerge) { + commitMessage += ' [skip ci]'; + } + await exec(`git commit -m "${commitMessage}"`); await exec(`git push --set-upstream ${remote} ${source}`); } From 6d7c09d989862c88494ba0abae22b232d570a71c Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Wed, 9 Mar 2022 19:50:37 +0000 Subject: [PATCH 10/14] slight docstring tweaks --- bin/cml/pr.js | 2 +- src/drivers/bitbucket_cloud.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/cml/pr.js b/bin/cml/pr.js index 2e5b731cc..2c867ee3e 100755 --- a/bin/cml/pr.js +++ b/bin/cml/pr.js @@ -22,7 +22,7 @@ exports.builder = (yargs) => autoMerge: { type: 'boolean', description: - 'If enabled, PR/MR will be marked for automatic merging (doesnt work on BitBucket).' + 'Mark the PR/MR for automatic merging after tests pass (unsupported by Bitbucket).' }, remote: { type: 'string', diff --git a/src/drivers/bitbucket_cloud.js b/src/drivers/bitbucket_cloud.js index f123c729d..6946a2a01 100644 --- a/src/drivers/bitbucket_cloud.js +++ b/src/drivers/bitbucket_cloud.js @@ -112,7 +112,7 @@ class BitbucketCloud { if (autoMerge) { throw new Error( - "Bitbucket Cloud doesn't allow Auto-Merging via API. Interested in this feature? Leave a thumbs-up here: TODO ADD A LINK" + "Auto-merging is unsupported by Bitbucket Cloud. See https://jira.atlassian.com/browse/BCLOUD-14286" ); } From 997e5030c5ebc7a2b32479c7181f15dd312c90a3 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Wed, 9 Mar 2022 21:41:43 +0000 Subject: [PATCH 11/14] linting --- src/drivers/bitbucket_cloud.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/bitbucket_cloud.js b/src/drivers/bitbucket_cloud.js index 6946a2a01..acbba0abc 100644 --- a/src/drivers/bitbucket_cloud.js +++ b/src/drivers/bitbucket_cloud.js @@ -112,7 +112,7 @@ class BitbucketCloud { if (autoMerge) { throw new Error( - "Auto-merging is unsupported by Bitbucket Cloud. See https://jira.atlassian.com/browse/BCLOUD-14286" + 'Auto-merging is unsupported by Bitbucket Cloud. See https://jira.atlassian.com/browse/BCLOUD-14286' ); } From 94e36ccc3bbe5cbcb57c5996c6137df22fb992d4 Mon Sep 17 00:00:00 2001 From: DavidGOrtega Date: Tue, 1 Mar 2022 00:27:23 +0100 Subject: [PATCH 12/14] fix test gl token len --- src/drivers/gitlab.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/drivers/gitlab.test.js b/src/drivers/gitlab.test.js index 04ba89768..d8ccb148d 100644 --- a/src/drivers/gitlab.test.js +++ b/src/drivers/gitlab.test.js @@ -40,11 +40,10 @@ describe('Non Enviromental tests', () => { test('Runner token', async () => { const output = await client.runnerToken(); - - expect(output.length).toBe(20); + expect(output.length >= 20).toBe(true); }); - test.skip('updateGitConfig', async () => { + test('updateGitConfig', async () => { const client = new GitlabClient({ repo: 'https://gitlab.com/test/test', token: 'dXNlcjpwYXNz' From 5294d1a1aec2f7b0fd76ae4f2feac208df5443b8 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Wed, 9 Mar 2022 21:59:17 +0000 Subject: [PATCH 13/14] fix tests --- bin/cml/pr.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/cml/pr.test.js b/bin/cml/pr.test.js index 1f4aae33c..cda536051 100644 --- a/bin/cml/pr.test.js +++ b/bin/cml/pr.test.js @@ -15,6 +15,8 @@ describe('CML e2e', () => { --log Maximum log level [string] [choices: \\"error\\", \\"warn\\", \\"info\\", \\"debug\\"] [default: \\"info\\"] --md Output in markdown format [](url). [boolean] + --auto-merge Mark the PR/MR for automatic merging after tests pass + (unsupported by Bitbucket). [boolean] --remote Sets git remote. [string] [default: \\"origin\\"] --user-email Sets git user email. [string] [default: \\"olivaw@iterative.ai\\"] --user-name Sets git user name. [string] [default: \\"Olivaw[bot]\\"] From a44d3c1850eba4192763fa310e39a7bfb60a6ba2 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 15 Mar 2022 10:46:36 +0100 Subject: [PATCH 14/14] refactor: destructured opts --- src/drivers/github.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/drivers/github.js b/src/drivers/github.js index 9b6ab24b6..48c6d63c0 100644 --- a/src/drivers/github.js +++ b/src/drivers/github.js @@ -343,10 +343,10 @@ class Github { } /** - * @param {string} branch + * @param {{ branch: string }} opts * @returns {Promise} */ - async isProtected(branch) { + async isProtected({ branch }) { const octo = octokit(this.token, this.repo); const { owner, repo } = this.ownerRepo(); try { @@ -394,7 +394,7 @@ class Github { const { owner, repo } = this.ownerRepo(); const settingsUrl = `https://github.com/${owner}/${repo}/settings`; - const isProtected = await this.isProtected(base); + const isProtected = await this.isProtected({ branch: base }); if (!isProtected) { throw new Error( `Enabling Auto-Merge failed. Please set up branch protection and add "required status checks" for branch '${base}': ${settingsUrl}/branches`