alan-turing-institute / whatwhat

A reimagining of nowwhat in OCaml
MIT License
0 stars 0 forks source link

Increase laziness #84

Closed yongrenjie closed 1 year ago

yongrenjie commented 1 year ago

Configuration is loaded all the time, even when just doing whatwhat --help

https://github.com/alan-turing-institute/whatwhat/blob/dbe85a0b39e2436e28b12d0134b4beff8f73a8f5/lib/config.ml#L91-L114

Same with fetching GitHub users. This causes, for example, whatwhat export-team to fail if a GitHub token isn't provided (even though the action obviously doesn't need a token).

https://github.com/alan-turing-institute/whatwhat/blob/dbe85a0b39e2436e28b12d0134b4beff8f73a8f5/lib/githubRaw.ml#L84-L102

yongrenjie commented 1 year ago

I think, for the Config module, load_settings needs to read them in as options, and then the get_... functions need to essentially do Option.get (or to be cleaner, pattern matching)?

all_users is more difficult, it's something I tried to solve before but couldn't figure out how to. The below works in terms of evaluation semantics, but unfortunately gives an error: Nested calls to Lwt_main.run are not allowed. I guess it would work if we didn't use cohttp_lwt for that call...? :/


let all_users = lazy (...)
let get_all_users () = Lazy.force all_users
... (get_all_users ()) ...`
yongrenjie commented 1 year ago

... Solved by making all_users return a promise instead, because promises are inherently lazy and cached (you can't resolve the same promise twice).

https://ocsigen.org/lwt/latest/api/Lwt (emphasis mine)

  • A promise might not have a value yet. A promise in this state is called a pending promise.
  • Writing a value into a promise is called resolving it. A promise with a value is called a resolved promise.
  • Each promise can be resolved only once. After a promise has a value, the promise is immutable.
  • It's possible to attach callbacks to a promise. They will run when the promise has a value, i.e. is resolved. If the promise is already resolved when a callback is attached, the callback is run (almost) right away. If the promise is pending, the callback is put into a list and waits.
yongrenjie commented 1 year ago

Re. configuration, the load_settings function is already forgiving in that it accepts None values. However, the Config.get_... functions are not so forgiving because they raise an exception if the value is None.

The only place where get_... is called is, funnily enough, inside all_users (which has since been renamed to get_all_users_async). To get around this, I wrapped the get... calls inside a Lwt thread, so that their execution is deferred until the get_all_users_async promise is itself resolved. This accomplishes the desired effect: dune exec -- whatwhat --help works perfectly even when the config and secret files are deleted entirely.

  let* github_repo_owner, github_repo_name =
    try Lwt.return (Config.get_github_repo_owner (), Config.get_github_repo_name ()) with
    | Config.MissingConfig s -> Lwt.fail (Config.MissingConfig s)
    | Config.MissingSecret s -> Lwt.fail (Config.MissingSecret s)
  in