Skip to content
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

Cmdliner composability #59

Open
rgrinberg opened this issue Jun 16, 2016 · 6 comments
Open

Cmdliner composability #59

rgrinberg opened this issue Jun 16, 2016 · 6 comments

Comments

@rgrinberg
Copy link
Owner

Opium has Cmdliner term/args for convenience but these are no longer useful when a user wants their own CLI. Perhaps we can still expose the terms/args Opium defines to make it easier for people to construct their own CLI's.

@NightBlues
Copy link

I don't know if this should be a separate issue...
Usage of App.run_command leads to

Fatal error: exception Unix.Unix_error(Unix.ECONNRESET, "read", "")
Raised by primitive operation at file "src/unix/lwt_bytes.ml", line 116, characters 38-80
Called from file "src/unix/lwt_unix.cppo.ml", line 483, characters 13-24

on connection reset by browser, that causes whole process to crash.
As I understand - thats because of lack of exception handling in
https://github.com/rgrinberg/opium/blob/master/opium/app.ml#L18
so maybe run_unix should be customizable/composable too?

@shonfeder
Copy link
Contributor

shonfeder commented Dec 24, 2019

afaict, there is also no way for the user to inspect the values that can be passed in from run_command. So, for instance, the CLI generated by run_command accepts flags for log levels and port, but there doesn't seem to be any way for the user to access these values to respond to them. I think this falls under the banner of making the generated commands more composable.

This leads to somewhat confusing behavior and idioms like

log_reporter ()
>>= fun r ->
Logs.set_reporter r ;
Logs.set_level (Some Logs.Info) ;
app

Apparently this is what we need to do to setup a logger, but then it seems that the logger must have it's log level hard coded, so I wonder I wonder what the log level CLI flags are for! :)

@anuragsoni
Copy link
Collaborator

@shonfeder Which log level flags are you referring to? Opium uses some Logs internally that can be enabled if a user specifies the debug level (via the debug flag) But those just control if opium will log anything via the logs library. As a library opium will never setup anything about what log level to actually filter on, or which reporter to use. That will be done in the actual application like you referred. I think this is something that I should document rather than just leave as an example.

I might start a mld doc to showcase some examples and pointing out some libraries that can be used with opium for common workflows.

@shonfeder
Copy link
Contributor

Ah yes, thanks @anuragsoni. I've been digging in to the source code to figure out what's going on here, and I see that the debug and verbose flags don't set logs levels, they just add middleware that will then emit log messages. So this would mean that a user who is employing run_command needs to hard code in the log level, as you have in the example there. So I guess this comes back to the extensibility and accessibility of command line arguments, or, better, of the values that are configured for the App.t. E.g., it seems that someone using the library should be able to check what port the app is configured to serve. This is easy to remedy, just by adding some accessor functions into the underlying App struct.

@anuragsoni
Copy link
Collaborator

Yeah, the log level filter and the log reporter will indeed be on the application to specify.
But that being said, exposing the cmdliner terms is a good idea as that'd let anyone create their own CLI on top of opium. I've done similar things when using the Command module from Core, and its a nice experience for anyone trying to reuse the arguments to construct their own CLI.

@aeghost
Copy link

aeghost commented May 20, 2022

I had the same problem, Opium Cmdliner got in confusion with my execution Framework, overriding my Cmdliner usage.

For what it is worth now, you can avoid the Opium Cmdliner call using the Opium.start function yourself.

let make_app (conf : MyConfiguration.t) : Opium.App.t =
  Opium.App.empty
  |> Opium.App.cmd_name conf.name
  |> Opium.App.port conf.port

let run_opium_server app =
  let open Lwt.Syntax in
  Lwt.async (fun () ->
    let* _server = Opium.App.start app in
    Lwt.return_unit);
  let forever, _ = Lwt.wait () in
  forever

let any_other_jobs () = Lwt.return_unit

let main () =
  let%lwt c = MyConfiguration.load () in
  let app = make_app c in 
  Lwt.join [run_opium_server app; any_other_jobs ()]

let () = Lwt_main.run @@ main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants