From f3e6a0a72f374a4cbbe3c7060313d89642a753d6 Mon Sep 17 00:00:00 2001 From: Jozef Mokry Date: Fri, 2 Jun 2023 17:22:47 +0100 Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.5-beta.1 --- spr/src/commands/land.rs | 39 ++++++++++++++++++--------------------- spr/src/github.rs | 32 +++++++++++++++++++++++++++++++- spr/src/utils.rs | 27 ++++++++++++++++++++++++++- 3 files changed, 75 insertions(+), 23 deletions(-) diff --git a/spr/src/commands/land.rs b/spr/src/commands/land.rs index 22f113e..23bfad3 100644 --- a/spr/src/commands/land.rs +++ b/spr/src/commands/land.rs @@ -11,8 +11,8 @@ use std::{io::Write, process::Stdio, time::Duration}; use crate::{ error::{Error, Result, ResultExt}, github::{PullRequestState, PullRequestUpdate, ReviewStatus}, - message::build_github_body_for_merging, output::{output, write_commit_title}, + utils::do_with_retry, utils::run_command, }; @@ -303,26 +303,23 @@ pub async fn land( // used a base branch with this Pull Request or not. We have made sure the // target of the Pull Request is set to the master branch. So let GitHub do // the merge now! - octocrab::instance() - .pulls(&config.owner, &config.repo) - .merge(pull_request_number) - .method(octocrab::params::pulls::MergeMethod::Squash) - .title(pull_request.title) - .message(build_github_body_for_merging(&pull_request.sections)) - .sha(format!("{}", pr_head_oid)) - .send() - .await - .convert() - .and_then(|merge| { - if merge.merged { - Ok(merge) - } else { - Err(Error::new(formatdoc!( - "GitHub Pull Request merge failed: {}", - merge.message.unwrap_or_default() - ))) - } - }) + + // Sometimes it takes a couple of tries to land a PR. Let's try a few times. + do_with_retry( + || { + gh.land_pull_request( + pull_request_number, + &pull_request, + pr_head_oid, + ) + }, + 5, + |_| { + output("❌", "Landing GitHub Pull Request failed, will retry in 1 second") + }, + Duration::from_secs(1), + ) + .await } Err(err) => Err(err), }; diff --git a/spr/src/github.rs b/spr/src/github.rs index 4be8c31..c00d14c 100644 --- a/spr/src/github.rs +++ b/spr/src/github.rs @@ -6,13 +6,15 @@ */ use graphql_client::{GraphQLQuery, Response}; +use indoc::formatdoc; use serde::Deserialize; use crate::{ error::{Error, Result, ResultExt}, git::Git, message::{ - build_github_body, parse_message, MessageSection, MessageSectionsMap, + build_github_body, build_github_body_for_merging, parse_message, + MessageSection, MessageSectionsMap, }, }; use std::collections::{HashMap, HashSet}; @@ -436,6 +438,34 @@ impl GitHub { .and_then(|sha| git2::Oid::from_str(&sha.oid).ok()), }) } + + pub async fn land_pull_request( + &self, + pull_request_number: u64, + pull_request: &PullRequest, + pull_request_head_oid: git2::Oid, + ) -> Result { + octocrab::instance() + .pulls(&self.config.owner, &self.config.repo) + .merge(pull_request_number) + .method(octocrab::params::pulls::MergeMethod::Squash) + .title(pull_request.title.clone()) + .message(build_github_body_for_merging(&pull_request.sections)) + .sha(format!("{}", pull_request_head_oid)) + .send() + .await + .convert() + .and_then(|merge| { + if merge.merged { + Ok(merge) + } else { + Err(Error::new(formatdoc!( + "GitHub Pull Request merge failed: {}", + merge.message.unwrap_or_default() + ))) + } + }) + } } #[derive(Debug, Clone)] diff --git a/spr/src/utils.rs b/spr/src/utils.rs index 19cd6d8..62c6a50 100644 --- a/spr/src/utils.rs +++ b/spr/src/utils.rs @@ -7,7 +7,7 @@ use crate::error::{Error, Result}; -use std::{io::Write, process::Stdio}; +use std::{future::Future, io::Write, process::Stdio, time::Duration}; use unicode_normalization::UnicodeNormalization; pub fn slugify(s: &str) -> String { @@ -58,6 +58,31 @@ pub async fn run_command(cmd: &mut tokio::process::Command) -> Result<()> { Ok(()) } +pub async fn do_with_retry( + f: F, + attempts: u64, + on_error: H, + sleep_time: Duration, +) -> Result +where + F: Fn() -> Fut, + Fut: Future>, + H: Fn(&Error) -> Result<()>, +{ + let mut last_error = None; + for _ in 0..attempts { + match f().await { + Ok(val) => return Ok(val), + Err(err) => { + on_error(&err)?; + tokio::time::sleep(sleep_time).await; + last_error = Some(err); + } + } + } + Err(last_error.unwrap()) +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope.