Reproducible builds #102
Reproducible builds #102
Conversation
|
The PR title is confusing. |
| @@ -301,6 +301,21 @@ struct | |||
| reply); | |||
| [] | |||
|
|
|||
| let single_unikernel albatross ~user_name ~unikernel_name = | |||
There was a problem hiding this comment.
could we name this differently? I'm a bit confused by "single_unikernel" and "unikernel_info_one".
| | Error msg -> | ||
| Logs.err (fun m -> m "error while communicating with albatross: %s" msg); | ||
| [] | ||
| | Ok (_hdr, `Success (`Unikernel_info unikernels)) -> unikernels |
There was a problem hiding this comment.
What about directly here constraining to a single unikernel... and return (Name.t * Unikernel.info) option instead of having to deal with that at the call sites?
| | Error (`Msg err) -> | ||
| Middleware.redirect_to_error | ||
| ~data:(`String ("Builder_web request: " ^ err)) | ||
| ~title:"Update Error" ~api_meth:false `Internal_server_error reqd () |
There was a problem hiding this comment.
Should we be more specific than "Update error"?
| | Error (`Msg err) -> | ||
| Middleware.redirect_to_error | ||
| ~data:(`String ("Builder_web request: " ^ err)) | ||
| ~title:"Update Error" ~api_meth:false `Internal_server_error |
There was a problem hiding this comment.
Same as above. It would help to provide some context.
| | Error (`Msg err) -> | ||
| Middleware.redirect_to_error | ||
| ~data:(`String ("Builder_web request: " ^ err)) | ||
| ~title:"Update Error" ~api_meth:false `Internal_server_error |
| >>= fun unikernels -> | ||
| if List.length unikernels > 0 then | ||
| let name, unikernel = List.hd unikernels in | ||
| let job_response_body = ref "" in |
There was a problem hiding this comment.
is it possible to have a different API than passing an (empty) reference cell to the send_request?
| ~title:"Update Error" ~api_meth:false | ||
| `Internal_server_error reqd () | ||
| | Ok latest_job_data -> ( | ||
| match |
There was a problem hiding this comment.
the match if done on boolean values always looks strange to me.
Why not use if?
| String.equal latest_job_data.uuid current_job_data.uuid | ||
| with | ||
| | true -> | ||
| Logs.warn (fun m -> |
There was a problem hiding this comment.
not sure about the log level, warn feels a bit strange.
| a_href | ||
| ("/unikernel/update/" | ||
| ^ Option.value ~default:"" | ||
| (Vmm_core.Name.name u_name)); |
There was a problem hiding this comment.
I don't quite understand this Option here -- how can a unikernel_single be done where there's u_name being None?
| @@ -93,6 +97,350 @@ module Status = struct | |||
| |> Yojson.Basic.to_string | |||
| end | |||
|
|
|||
| module Builder_web = struct | |||
There was a problem hiding this comment.
should that better be in a separate file -- builder_web.ml?
| } | ||
|
|
||
| type duniverse = { name : string; value_ : string } | ||
| type duniverse_detailed_diff = { name : string } |
There was a problem hiding this comment.
that's pretty verbose -- can we remove this type (and use string instead)?
| left : duniverse list; | ||
| right : duniverse list; | ||
| detailed_diff : duniverse_detailed_diff list; | ||
| } |
There was a problem hiding this comment.
I don't quite understand why these type definitions differ from builder-web -- are the types in builder-web not ergonomic enough? should we separate the opamdiff subpackage into a custom opam package and depend on that in mollymawk (to share the type definitions and avoid code duplication)?
| let send_request url response_body = | ||
| let body_f _ acc chunk = | ||
| response_body := !response_body ^ chunk; | ||
| Lwt.return acc | ||
| in | ||
| Http_lwt_client.request ~follow_redirect:true | ||
| ~headers:[ ("Accept", "application/json") ] | ||
| url body_f () | ||
| >>= function | ||
| | Error (`Msg err) -> Lwt.return (Error (`Msg err)) | ||
| | Ok (resp, ()) -> ( | ||
| match resp.Http_lwt_client.status with | ||
| | `OK -> Lwt.return (Ok response_body) | ||
| | _ -> | ||
| Lwt.return | ||
| (Error | ||
| (`Msg (Format.asprintf "%a" Http_lwt_client.pp_response resp))) | ||
| ) |
There was a problem hiding this comment.
| let send_request url response_body = | |
| let body_f _ acc chunk = | |
| response_body := !response_body ^ chunk; | |
| Lwt.return acc | |
| in | |
| Http_lwt_client.request ~follow_redirect:true | |
| ~headers:[ ("Accept", "application/json") ] | |
| url body_f () | |
| >>= function | |
| | Error (`Msg err) -> Lwt.return (Error (`Msg err)) | |
| | Ok (resp, ()) -> ( | |
| match resp.Http_lwt_client.status with | |
| | `OK -> Lwt.return (Ok response_body) | |
| | _ -> | |
| Lwt.return | |
| (Error | |
| (`Msg (Format.asprintf "%a" Http_lwt_client.pp_response resp))) | |
| ) | |
| let send_request url = | |
| let body = "" in | |
| let body_f _ acc chunk = | |
| Lwt.return (acc ^ chunk) | |
| in | |
| Http_lwt_client.request ~follow_redirect:true | |
| ~headers:[ ("Accept", "application/json") ] | |
| url body_f body | |
| >>= function | |
| | Error (`Msg err) -> Lwt.return (Error (`Msg err)) | |
| | Ok (resp, body) -> ( | |
| match resp.Http_lwt_client.status with | |
| | `OK -> Lwt.return (Ok body) | |
| | _ -> | |
| Lwt.return | |
| (Error | |
| (`Msg (Format.asprintf "%a" Http_lwt_client.pp_response resp))) | |
| ) |
this way we avoid the reference cell, and deliver the body as output
| let bytes_to_megabytes bytes = | ||
| let megabytes = float_of_int bytes /. (1024.0 *. 1024.0) in | ||
| Printf.sprintf "%.2f" megabytes |
There was a problem hiding this comment.
should we write the "MB" as well to the string?
Related to #54
This PR checks if there is an update to a unikernel from
builds.robur.coopand if there's an update it displays the differences between the current unikernel and the update.At the moment, this PR only displays the diff and doesn't update the unikernel binary (yet).