From 7cb81a18ddb6059fef1e8b91f191263c5f27695d Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Wed, 20 Sep 2023 13:10:29 +0200 Subject: Update attention set behavior and un-WIPing changes Fixes: QTQAINFRA-5797 Change-Id: I12415e774715c97d8e4a01e5836609e8db79a770 Reviewed-by: Daniel Smith --- .../cherry-pick_automation/gerritRESTTools.js | 69 +++++++++++++++++++--- .../integration_monitor/integration_monitor.js | 2 +- .../cherry-pick_automation/requestProcessor.js | 2 +- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/scripts/gerrit/cherry-pick_automation/gerritRESTTools.js b/scripts/gerrit/cherry-pick_automation/gerritRESTTools.js index 7adf4ba2..a67e89f3 100644 --- a/scripts/gerrit/cherry-pick_automation/gerritRESTTools.js +++ b/scripts/gerrit/cherry-pick_automation/gerritRESTTools.js @@ -661,11 +661,13 @@ function setChangeReviewers(parentUuid, fullChangeID, reviewers, customAuth, cal callback(failedItems); let project = /^((?:\w+-?)+(?:%2F|\/)(?:-?\w)+)/.exec(fullChangeID).pop(); let branch = /~(.+)~/.exec(fullChangeID).pop(); + let doneCount = 0; function postReviewer(reviewer) { checkAccessRights( parentUuid, project, branch, reviewer, "read", customAuth, function (success, data) { if (!success) { + doneCount++; logger.log(`Dropping reviewer ${reviewer} from cherry-pick to ${ branch} because they can't view it.`, "info", parentUuid); logger.log(`Reason: ${data}`, "debug", parentUuid); @@ -683,8 +685,14 @@ function setChangeReviewers(parentUuid, fullChangeID, reviewers, customAuth, cal `Success adding ${reviewer} to ${fullChangeID}\n${response.data}`, "info", parentUuid ); + doneCount++; + if (doneCount == reviewers.length) + callback(failedItems); }) .catch(function (error) { + doneCount++; + if (doneCount == reviewers.length) + callback(failedItems); if (error.response) { // The request was made and the server responded with a status code // that falls out of the range of 2xx @@ -708,8 +716,49 @@ function setChangeReviewers(parentUuid, fullChangeID, reviewers, customAuth, cal // Not possible to batch reviewer adding into a single request. Iterate through // the list instead. - reviewers.forEach(postReviewer); - callback(failedItems); + setReadyForReview(parentUuid, fullChangeID, customAuth, function () { + // Even if setting Ready fails, still attempt to post the reviewers. + reviewers.forEach(postReviewer); + }); +} + +exports.setReadyForReview = setReadyForReview; +function setReadyForReview(parentUuid, fullChangeID, customAuth, callback) { + let url = `${gerritBaseURL("changes")}/${fullChangeID}/ready`; + axios({ method: "post", url: url, data: {}, auth: customAuth || gerritAuth }) + .then(function (response) { + logger.log(`Successfully set ready for review on change ${fullChangeID}`, "verbose", parentUuid); + callback(true, undefined); + }) + .catch(function (error) { + if (error.response) { + // 409 is expected if the change is not WIP. + if (error.response.status == 409) { + logger.log(`Change ${fullChangeID} is not WIP.`, "debug", parentUuid); + callback(true, undefined); + } else { + // The request was made and the server responded with a status code + // that falls out of the range of 2xx + logger.log( + `An error occurred in POST to "${url}". Error ${error.response.status}: ${ + error.response.data}`, + "error", parentUuid + ); + callback(false, error.response.status); + } + } else if (error.request) { + // The request was made but no response was received + callback(false, "retry"); + } else { + // Something happened in setting up the request that triggered an Error + logger.log( + `UNKNOWN ERROR while setting ready for review on ${ + fullChangeID}: ${safeJsonStringify(error)}`, + "error", parentUuid + ); + callback(false, error.message); + } + }); } // Copy reviewers from one change ID to another @@ -758,13 +807,19 @@ function locateDefaultAttentionUser(uuid, cherryPickChange, uploader, callback) } if (originalApprover && originalApprover != uploader) { // The approver from the original change should be able to help. - checkAccessRights(uuid, cherryPickChange.project, cherryPickChange.branch, originalApprover, + let project = typeof cherryPickChange.project === "string" + ? cherryPickChange.project + : cherryPickChange.change.project; + let branch = cherryPickChange.branch + ? cherryPickChange.branch + : cherryPickChange.change.branch; + checkAccessRights(uuid, project, branch, originalApprover, "read", undefined, (canRead) => { if (canRead) callback(originalApprover); else - tryFallback(); + tryFallback(project, branch); } ); } else { @@ -772,7 +827,7 @@ function locateDefaultAttentionUser(uuid, cherryPickChange, uploader, callback) } } - function tryFallback() { + function tryFallback(project, branch) { // This insane regex is the same as used in the commit message sanitizer, // So it should always find the right footer which references the // picked-from sha. @@ -794,8 +849,8 @@ function locateDefaultAttentionUser(uuid, cherryPickChange, uploader, callback) if (uploader != originalAuthor) { // Add the author of the original change's final patchset to the // attention set of the cherry-pick. - checkAccessRights(uuid, cherryPickChange.project, cherryPickChange.branch, - originalApprover, "read", undefined, + checkAccessRights(uuid, project, branch, + originalAuthor, "read", undefined, (canRead) => { if (canRead) callback(originalAuthor); diff --git a/scripts/gerrit/cherry-pick_automation/plugin_bots/integration_monitor/integration_monitor.js b/scripts/gerrit/cherry-pick_automation/plugin_bots/integration_monitor/integration_monitor.js index e92c8150..367d9efe 100644 --- a/scripts/gerrit/cherry-pick_automation/plugin_bots/integration_monitor/integration_monitor.js +++ b/scripts/gerrit/cherry-pick_automation/plugin_bots/integration_monitor/integration_monitor.js @@ -87,7 +87,7 @@ class integration_monitor { req.change.fullChangeID = req.fullChangeID // Tack on the full change ID so it gets used _this.doAddToAttentionSet(req, author, "Change author", (success) => { if (!success) { - gerritTools.locateDefaultAttentionUser(req.uuid, req.change, req.change.patchSet.uploader.email, + gerritTools.locateDefaultAttentionUser(req.uuid, req.change, req.change.owner.email, (user, fallbackId) => { if (user == "copyReviewers") gerritTools.copyChangeReviewers(req.uuid, fallbackId, req.change.fullChangeID); diff --git a/scripts/gerrit/cherry-pick_automation/requestProcessor.js b/scripts/gerrit/cherry-pick_automation/requestProcessor.js index 6ebed37d..e827d505 100644 --- a/scripts/gerrit/cherry-pick_automation/requestProcessor.js +++ b/scripts/gerrit/cherry-pick_automation/requestProcessor.js @@ -1305,7 +1305,7 @@ class requestProcessor extends EventEmitter { function (success, data) { if (!success) { _this.logger.log( - `Failed to add "${safeJsonStringify(parentJSON.change.owner)}" to the` + `Failed to add "${user}" to the` + ` attention set of ${cherryPickJSON.id}\n` + `Reason: ${safeJsonStringify(data)}`, "error", parentJSON.uuid -- cgit v1.2.3