-
Notifications
You must be signed in to change notification settings - Fork 459
Various refactorings in the RPC system #12580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fa952fd
4c6dcec
ec18fd7
57abe9f
ef8f1b1
c6a9f88
218be7f
5be0c8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,24 @@ | ||
| open Import | ||
|
|
||
| let exec () = | ||
| let open Fiber.O in | ||
| let where = Rpc.Rpc_common.active_server_exn () in | ||
| let module Client = Dune_rpc_client.Client in | ||
| let+ errors = | ||
| let* connect = Client.Connection.connect_exn where in | ||
| Dune_rpc_impl.Client.client | ||
| connect | ||
| (Dune_rpc_private.Initialize.Request.create | ||
| ~id:(Dune_rpc_private.Id.make (Sexp.Atom "diagnostics_cmd"))) | ||
| ~f:(fun cli -> | ||
| let* decl = | ||
| Client.Versioned.prepare_request cli Dune_rpc_private.Public.Request.diagnostics | ||
| in | ||
| match decl with | ||
| | Error e -> raise (Dune_rpc_private.Version_error.E e) | ||
| | Ok decl -> Client.request cli decl ()) | ||
| in | ||
| match errors with | ||
| | Ok errors -> | ||
| List.iter errors ~f:(fun err -> | ||
| Console.print_user_message (Dune_rpc.Diagnostic.to_user_message err)) | ||
| | Error e -> Rpc.Rpc_common.raise_rpc_error e | ||
| ;; | ||
|
|
||
| let info = | ||
| let doc = "Fetch and return errors from the current build." in | ||
| Cmd.info "diagnostics" ~doc | ||
| ;; | ||
|
|
||
| let term = | ||
| let+ (builder : Common.Builder.t) = Common.Builder.term in | ||
| Rpc.Rpc_common.client_term builder exec | ||
| Rpc.Rpc_common.client_term builder (fun () -> | ||
| let open Fiber.O in | ||
| let+ errors = | ||
| Rpc.Rpc_common.fire_request | ||
| ~name:"diagnostics_cmd" | ||
| ~wait:false | ||
| builder | ||
| Dune_rpc_private.Procedures.Public.diagnostics | ||
| () | ||
| in | ||
| List.iter errors ~f:(fun err -> | ||
| Console.print_user_message (Dune_rpc.Diagnostic.to_user_message err))) | ||
| ;; | ||
|
|
||
| let command = Cmd.v info term |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,6 @@ val active_server_exn : unit -> Dune_rpc.Where.t | |
| (** Raise an RPC response error. *) | ||
| val raise_rpc_error : Dune_rpc.Response.Error.t -> 'a | ||
|
|
||
| (** Make a request and raise an exception if the preparation for the request | ||
| fails in any way. Returns an [Error] if the response errors. *) | ||
| val request_exn | ||
| : Dune_rpc_client.Client.t | ||
| -> ('a, 'b) Dune_rpc.Decl.request | ||
| -> 'a | ||
| -> ('b, Dune_rpc.Response.Error.t) result Fiber.t | ||
|
|
||
| (** Cmdliner term for a generic RPC client. *) | ||
| val client_term : Common.Builder.t -> (unit -> 'a Fiber.t) -> 'a | ||
|
|
||
|
|
@@ -38,26 +30,27 @@ val fire_request | |
| -> Common.Builder.t | ||
| -> ('a, 'b) Dune_rpc.Decl.request | ||
| -> 'a | ||
| -> ('b, Dune_rpc.Response.Error.t) result Fiber.t | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come we don't want to know whether the RPC resulted in error anymore? It seems to me that it would be useful, as I can imagine we could recover from sending requests in some cases.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a pragmatic change: every single user of this function had the exact same error handling code, so I just factored it in directly. |
||
| -> 'b Fiber.t | ||
|
|
||
| (** Send a notification to the RPC server. If [wait], it will poll forever until a server is listening. | ||
| Should be scheduled by a scheduler that does not come with a RPC server on its own. | ||
|
|
||
| [warn_forwarding] defaults to true, warns the user that since a RPC server is running, some arguments are ignored. | ||
| [lock_held_by] defaults to [Unknown], is only used to allow error messages to print the PID. *) | ||
| val fire_notification | ||
| : name:string | ||
| -> wait:bool | ||
| -> ?warn_forwarding:bool | ||
| -> ?lock_held_by:Dune_util.Global_lock.Lock_held_by.t | ||
| -> Common.Builder.t | ||
| -> 'a Dune_rpc.Decl.notification | ||
| -> 'a | ||
| -> unit Fiber.t | ||
|
|
||
| val wrap_build_outcome_exn | ||
| : print_on_success:bool | ||
| -> ('a | ||
| -> (Dune_rpc.Build_outcome_with_diagnostics.t, Dune_rpc.Response.Error.t) result | ||
| Fiber.t) | ||
| -> 'a | ||
| -> Dune_rpc.Build_outcome_with_diagnostics.t | ||
|
Comment on lines
43
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see we don't always use this when we call
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new type should make the decision to use or not to use this function clearer: |
||
| -> unit | ||
| -> unit Fiber.t | ||
|
|
||
| (** Warn the user that since a RPC server is running, some arguments are ignored. *) | ||
| val warn_ignore_arguments : Dune_util.Global_lock.Lock_held_by.t -> unit | ||
|
|
||
| (** Schedule a fiber to run via RPC, wrapping any errors. *) | ||
| val run_via_rpc | ||
| : common:Common.t | ||
| -> config:Dune_config_file.Dune_config.t | ||
| -> ('a | ||
| -> (Dune_rpc.Build_outcome_with_diagnostics.t, Dune_rpc.Response.Error.t) result | ||
| Fiber.t) | ||
| -> 'a | ||
| -> unit | ||
Uh oh!
There was an error while loading. Please reload this page.