Improve lwt_pool introduction docs with examples

[bobbypriam]

I believe this change will improve the clarity of the module overview with a more flowing paragraphs and examples.

Feel free to bash this (or suggest improvements) if you disagree. 😄

[bobbypriam]

I also just noticed there is still a reference to threads instead of promises in the documentation for wait_queue_length. Do you think we should change it too?

[aantron]

Thanks! It looks good to me. The only thing I'd optionally suggest is not having an Lwt-aware Db module, because that leaves open the question of whether the module has to be Lwt-aware (it doesn't). And, of course, it would be best if the example was something runnable, but I'm not sure how easy it is to come up with a dummy "real" replacement for Db.

Yes, we should change threads in wait_queue_length, if you'd like to do it now, please :) However, changing it to promises doesn't make sense, because promises is not a synonym for threads and can't be used as threads is used in that sentence. It would have to be changed to something like "requests." Ideally, that would be part of a coherent terminology, e.g. use would "request" an element of the pool, etc.

[aantron]

(and disregard Travis, there has been an upstream problem with Homebrew's Python for a while now, I haven't had time to look into it)

[hannesm]

@aantron FWIW ocaml/ocaml-ci-scripts#225 got merged to work around the python issue on macos

[aantron]

Thanks @hannesm. I fixed it in master using the PR you linked as a guide. Based on comments in Homebrew/homebrew-core#26358, this is not a temporary issue.

[bobbypriam]

I don't mind changing it, but I was tempted to say Lwt-aware due to the fact that the creation function, dispose, and use needed to return promises.

"Requests" sounds good. I think I'll also add a note saying that in case the pool is exhausted, use will wait for available resource.

[bobbypriam]

Okay, so for now I have removed the "Lwt-aware" to make it more straightforward. I think the goal of the current example is to give a birds-eye view of how the common pieces may fit together in the real-world, and IMO forcing it to be a self-contained, runnable example would need us to write a mock Db and make it more difficult to see the main point at a glance.

Please let me know if you still have any concerns. I'm also fine if you're okay with merging it as-is.

[aantron]

Thanks!

These docs are a definite big improvement, so I want to be clear I am just talking about very minor issues with "Lwt-aware" :) Because the Lwt-awareness is not a property of Db, but only necessary for communicating with Lwt_pool, one could use any non-Lwt-aware library and insert Lwt.return in the right place. In super, super nitpicky mode, I would write the example with a separated-out Lwt.return, because it is easier for the reader to read such a broken-out example, and then realize "if my functions already happen to return promises, I don't need to do a separate Lwt.return," than to realize the Lwt awareness is optional, after wondering if there is maybe some hidden reason why the user's functions "have" to know about Lwt. This is probably especially important for newer readers whose understanding of what Lwt "really" is and how promises really work is still a bit shaky, e.g. if they are still not sure that Lwt might be some kind of special runtime that executes functions in a special way, etc. I know I was in that category for a while :) Though back then the "threads" language being used everywhere made the problem much worse.

[bobbypriam]

Thank you!

And yes, you made great points, I didn't think of that. While I'm at it, do you mind if I open another PR to put Lwt.returns on the examples?

[aantron]

Of course not, please do :)