From 541cf26875c0ffa393a9653705f9504e8211021e Mon Sep 17 00:00:00 2001 From: Francois Kritzinger Date: Wed, 23 Oct 2024 10:20:48 +0200 Subject: handle_pull_request(): Create pre-check CI request --- mod/ci-common.hxx | 3 ++ mod/mod-ci-github.cxx | 92 ++++++++++++++++++++++++++++----------------------- mod/mod-ci-github.hxx | 19 ----------- 3 files changed, 53 insertions(+), 61 deletions(-) (limited to 'mod') diff --git a/mod/ci-common.hxx b/mod/ci-common.hxx index 4bd63c2..c03abf8 100644 --- a/mod/ci-common.hxx +++ b/mod/ci-common.hxx @@ -101,6 +101,9 @@ namespace brep // // Note: should be called out of the database transaction. // + // @@ TMP Shouldn't the comments mention that if tenant_service.id is an + // empty string then the generated tenant id will be used? + // enum class duplicate_tenant_mode {fail, ignore, replace, replace_archived}; enum class duplicate_tenant_result {created, ignored, replaced}; diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index b03298c..6c0bb72 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -493,8 +493,15 @@ namespace brep // Create an unloaded CI request. // + // Note: use no delay since we need to (re)create the synthetic conclusion + // check run as soon as possible. + // // Note that we use the create() API instead of start() since duplicate - // management is no available in start(). + // management is not available in start(). + // + // After this call we will start getting the build_unloaded() + // notifications until (1) we load the request, (2) we cancel it, or (3) + // it gets archived after some timeout. // auto pr (create (error, warn, @@ -524,6 +531,8 @@ namespace brep // High-level description of pull request (PR) handling // + // @@ TODO Update these comments. + // // - Some GitHub pull request terminology: // // - Fork and pull model: Pull requests are created in a forked @@ -681,27 +690,56 @@ namespace brep l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); + // @@ TMP Regarding pushes to PR head branches ("synchronize"): If a + // remote PR head branch is not behind base, wouldn't we want to cancel + // existing CIs and restart? This is the only event we'll see when the + // user clicks the "update PR head branch with base" button. + // + + // @@ TMP We can already determine whether the PR is local or remote so we + // could pass `kind` to the service_data constructor. + + // @@ TODO Serialize the new service_data fields! + // + // Note that check_sha will be set later, in build_unloaded_pre_check(). + // service_data sd (warning_success, move (iat->token), iat->expires_at, pr.installation.id, move (pr.repository.node_id), - pr.pull_request.head_sha /* report_sha */, - pr.repository.clone_url, + move (pr.pull_request.head_sha) /* report_sha */, + move (pr.repository.clone_url), pr.pull_request.number); - // Create unloaded CI request. Cancel the existing CI request first if the - // head branch has been updated (action is `synchronize`). + assert (sd.pre_check); + + // Create an unloaded CI request for the pre-check phase (during which we + // wait for the PR's merge commit and behindness to become available). // - // After this call we will start getting the build_unloaded() - // notifications until (1) we load the request, (2) we cancel it, or (3) - // it gets archived after some timeout. + // Create with an empty service id so that the generated tenant id is used + // instead during the pre-check phase (so as not to clash with a proper + // service id for this head commit, potentially created in + // handle_check_suite()). // - bool cancel_first (pr.action == "synchronize"); + tenant_service ts ("", "ci-github", sd.json ()); - if (!create_pull_request_ci (error, warn, trace, - sd, pr.pull_request.node_id, - cancel_first)) + // Note: use no delay since we need to (re)create the synthetic conclusion + // check run as soon as possible. + // + // After this call we will start getting the build_unloaded() + // notifications -- which will be routed to build_unloaded_pre_check() -- + // until we cancel the request or it gets archived after some + // timeout. (Note that we never actually load this request, we always + // cancel it; see build_unloaded_pre_check() for details.) + // + if (!create (error, + warn, + &trace, + *build_db_, + move (ts), + chrono::seconds (30) /* interval */, + chrono::seconds (0) /* delay */)) { fail << "pull request " << pr.pull_request.node_id << ": unable to create unloaded CI request"; @@ -1881,36 +1919,6 @@ namespace brep }; } - bool ci_github:: - create_pull_request_ci (const basic_mark& error, - const basic_mark& warn, - const basic_mark& trace, - const service_data& sd, - const string& nid, - bool cf) const - { - // Cancel the existing CI request if asked to do so. Ignore failure - // because the request may already have been cancelled for other reasons. - // - if (cf) - { - if (!cancel (error, warn, &trace, *build_db_, "ci-github", nid)) - l3 ([&] {trace << "unable to cancel CI for pull request " << nid;}); - } - - // Create a new unloaded CI request. - // - tenant_service ts (nid, "ci-github", sd.json ()); - - // Note: use no delay since we need to (re)create the synthetic merge - // check run as soon as possible. - // - return create (error, warn, &trace, - *build_db_, move (ts), - chrono::seconds (30) /* interval */, - chrono::seconds (0) /* delay */).has_value (); - } - string ci_github:: details_url (const build& b) const { diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index e919065..8284f7f 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -91,25 +91,6 @@ namespace brep bool handle_pull_request (gh_pull_request_event, bool warning_success); - // Create an unloaded CI request for a pull request. If `cancel_first` is - // true, cancel its existing CI request first. - // - // Return true if an unloaded CI request was created. Ignore failure to - // cancel because the CI request may already have been cancelled for other - // reasons. - // - // After this call we will start getting the build_unloaded() - // notifications until (1) we load the request, (2) we cancel it, or (3) - // it gets archived after some timeout. - // - bool - create_pull_request_ci (const basic_mark& error, - const basic_mark& warn, - const basic_mark& trace, - const service_data&, - const string& pull_request_node_id, - bool cancel_first) const; - // Build a check run details_url for a build. // string -- cgit v1.1