diff options
| -rw-r--r-- | mod/mod-ci-github.cxx | 61 |
1 files changed, 30 insertions, 31 deletions
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index f6827da..7ede14f 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -671,17 +671,6 @@ namespace brep // 2. If the tenant is archived, then fail (re-create) both the check run // and the conclusion with appropriate diagnostics. // - // @@ TMP A PR might get blocked indefinitely by this. If the user - // re-runs a single CR the "re-run all check runs" option disappears - // from the UI. So the single re-run will keep failing but there - // will be no way to start a new CI from scratch. - // - // @@ Can we confirm that if we also fail the check run, then the - // re-run check suite becomes possible again? - // - // @@ Can confirm: failing the CR makes re-run check suite option - // re-appear. - // // 3. If the check run is in the queued state, then do nothing. // // 4. Re-create the check run in the queued state and the conclusion in @@ -961,17 +950,21 @@ namespace brep } // Request the rebuild and update service data. + // + bool race (false); // Callback function called by rebuild() to update the service data (but // only if the build is actually restarted). // - auto update_sd = [&error, &new_iat, + auto update_sd = [&error, &new_iat, &race, &cr, &bcr, &ccr] (const tenant_service& ts, build_state) -> optional<string> { // NOTE: this lambda may be called repeatedly (e.g., due to transaction // being aborted) and so should not move out of its captures. + race = false; // Reset. + service_data sd; try { @@ -985,11 +978,8 @@ namespace brep // Note that we again look by name in case node id got replaced by a // racing re-request. In this case, however, it's impossible to decide - // who won that race, so let's assume it's us since we are called last. - // - // @@ TMP Isn't the replaced node id situation basically the same as the - // one where rebuild() returns queued? In which case we could handle - // it the same way. + // who won that race, so let's fail the check suite to be on the safe + // side (in a sense, similar to the rebuild() returning queued below). // auto i (find_if ( sd.check_runs.begin (), sd.check_runs.end (), @@ -1006,6 +996,15 @@ namespace brep return nullopt; } + if (i->node_id && *i->node_id != cr.check_run.node_id) + { + // Keep the old conclusion node id to make sure any further state + // transitions are ignored. A bit of a hack. + // + race = true; + return nullopt; + } + *i = bcr; // Update with new node_id, state, state_synced. sd.conclusion_node_id = ccr.node_id; @@ -1026,25 +1025,16 @@ namespace brep // conclusion check run. Otherwise the build has been successfully // re-enqueued so do nothing further. // - if (bs && *bs != build_state::queued) + if (!race && bs && *bs != build_state::queued) return true; gq_built_result br; // Built result for both check runs. - if (!bs) // Archived - { - // The build has expired since we loaded the service data. Most likely - // the tenant has been archived. - // - br = make_built_result ( - result_status::error, warning_success, - "Unable to rebuild individual configuration: build has been archived"); - } - else // Re-enqueued. + if (race || bs) // Race or re-enqueued. { - // This build has been re-enqueued since we first loaded the service - // data. This could happen if the user clicked "re-run" multiple times - // and another handler won the rebuild() race. + // The re-enqueued case: this build has been re-enqueued since we first + // loaded the service data. This could happen if the user clicked + // "re-run" multiple times and another handler won the rebuild() race. // // However the winner of the check runs race cannot be determined. // @@ -1063,6 +1053,15 @@ namespace brep br = make_built_result (result_status::error, warning_success, "Unable to rebuild, try again"); } + else // Archived. + { + // The build has expired since we loaded the service data. Most likely + // the tenant has been archived. + // + br = make_built_result ( + result_status::error, warning_success, + "Unable to rebuild individual configuration: build has been archived"); + } // Try to update the conclusion check run even if the first update fails. // |