buildkite: call get_build with build url

thatportugueseguy · thatportugueseguy · commit 71cb37831f63 · 2025-03-12T12:17:27.000Z
diff --git a/lib/action.ml b/lib/action.ml
@@ -642,9 +642,8 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) (Buildkite_api :
                      ~failed_steps:Common.FailedStepSet.empty n channel;
                  ]
              | Failed ->
-               let get_build ~build_url = Buildkite_api.get_build ~ctx build_url in
                (* repo state is updated upon fetching new failed steps *)
-               (match%lwt new_failed_steps ~repo_state ~get_build n with
+               (match%lwt new_failed_steps ~repo_state ~get_build:(Buildkite_api.get_build ~ctx) n with
                | Error e -> action_error e
                | Ok failed_steps ->
                match FailedStepSet.is_empty failed_steps with
diff --git a/lib/api_local.ml b/lib/api_local.ml
@@ -238,13 +238,10 @@ module Buildkite : Api.Buildkite = struct
 
   [@@@warning "-27"]
   let get_build ?(cache : [ `Use | `Refresh ] option) ~ctx build_url =
-    match Re2.find_submatches_exn Util.Build.buildkite_org_pipeline_build_re build_url with
-    | exception _ -> failwith "Failed to parse Buildkite build url"
-    | [| Some _; Some org; Some pipeline; Some build_nr |] ->
-      let file = clean_forward_slashes (sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr) in
-      let url = Filename.concat buildkite_cache_dir file in
-      with_cache_file url Buildkite_j.get_build_res_of_string
-    | _ -> failwith "failed to get all build details from the notification. Is this a Buildkite notification?"
+    let* org, pipeline, build_nr = Lwt.return @@ Util.Build.get_org_pipeline_build' build_url in
+    let file = clean_forward_slashes (sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr) in
+    let url = Filename.concat buildkite_cache_dir file in
+    with_cache_file url Buildkite_j.get_build_res_of_string
 
   let get_build_branch ~ctx (n : Github_t.status_notification) =
     let* build_url = Lwt.return @@ Util.Build.get_build_url n in
diff --git a/lib/api_remote.ml b/lib/api_remote.ml
@@ -332,30 +332,29 @@ module Buildkite : Api.Buildkite = struct
 
   let get_build ?(cache : [ `Use | `Refresh ] = `Use) ~(ctx : Context.t) build_url =
     let* org, pipeline, build_nr = Lwt.return @@ Util.Build.get_org_pipeline_build' build_url in
-    let build_url = sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr in
+    let path = sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr in
     let build_key = cache_key org pipeline build_nr in
     let* build =
       let get () =
-        request_token_auth ~name:("get build details for #" ^ build_nr) ~ctx `GET build_url
+        request_token_auth ~name:("get build details for #" ^ build_nr) ~ctx `GET path
           Buildkite_j.get_build_res_of_string
       in
       match cache with
       | `Refresh -> get ()
       | `Use ->
       match Builds_cache.get builds_cache build_key with
-      | Some build -> Lwt.return_ok build
+      | Some build ->
+        log#info "Cache hit for build %s" build_key;
+        Lwt.return_ok build
       | None -> get ()
     in
     Builds_cache.set builds_cache build_key build;
     Lwt.return_ok build
 
   let get_build_branch ~(ctx : Context.t) (n : Github_t.status_notification) =
     let* org, pipeline, build_nr = Lwt.return @@ Util.Build.get_org_pipeline_build n in
-    let build_url = sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr in
+    let* build_url = Lwt.return @@ Util.Build.get_build_url n in
     let map_branch { Buildkite_t.branch; _ } : Github_t.branch = { name = branch } in
-    match Builds_cache.get builds_cache (cache_key org pipeline build_nr) with
-    | Some { Buildkite_t.branch; _ } -> Lwt.return_ok ({ name = branch } : Github_t.branch)
-    | None ->
-      log#info "Fetching branch details for build %s in pipeline %s" build_nr pipeline;
-      get_build ~ctx build_url |> Lwt_result.map map_branch
+    log#info "Fetching branch details for %s" (cache_key org pipeline build_nr);
+    get_build ~cache:`Use ~ctx build_url |> Lwt_result.map map_branch
 end
diff --git a/lib/util.ml b/lib/util.ml
@@ -349,9 +349,8 @@ module Webhook = struct
     let* org, pipeline, build_nr = Lwt.return @@ Build.get_org_pipeline_build' n.build.web_url in
     let repo_key = repo_key org pipeline in
     let get_failed_steps () =
-      log#info "Fetching failed steps for org=%s, pipeline=%s, build=%s" org pipeline build_nr;
-      let build_url = Printf.sprintf "organizations/%s/pipelines/%s/builds/%s" org pipeline build_nr in
-      let* (build : Buildkite_t.get_build_res) = get_build ~build_url in
+      log#info "Fetching failed steps for build %s/%s/%s" org pipeline build_nr;
+      let* (build : Buildkite_t.get_build_res) = get_build n.build.web_url in
       let to_failed_step (job : Buildkite_t.job) = { Buildkite_t.name = job.name; build_url = job.web_url } in
       Lwt.return_ok @@ (Build.filter_failed_jobs build.jobs |> List.map to_failed_step |> Common.FailedStepSet.of_list)
     in
