Consider adding an implicit ExecutionContext to HttpDriver.send()

[easel]

I ran into an interesting issue writing tests with scalatest.

In short, scalatest by default doesn't use the global implicit ExecutionContext.

The current JVM http driver implementation does use the default ExecutionContext.

This causes weird side effects. Here's some background I found on the scalatest side https://github.com/scalatest/scalatest/issues/910

In general, I think it may be beneficial to introduce an implicit ec: ExecutionContext to the send() method and eliminate the global import. I'm not entirely positive, since with some backends it won't be necessary, but for the cases where it is, being unable to control the execution context is a pretty significant issue.

This will come up in high-load situations where one might want to separate thread pools for external http requests, etc as well.

[hmil]

I have to admit that I am not extremely familiar with Scala's execution context. How important do you think this is? Adding an extra parameter (even if implicit) will break backwards compatibility and make the API specific to an environment. The first is not a big problem as I plan to bundle all short-term features with breaking changes in v2. The later however is not acceptable. There must be a more subtle way to pass the execution context such that a full-js user doesn't have to worry about it at all.

[easel]

Well, the execution context is a pretty big deal since we're dealing with futures. Basically, the execution context is how you tell scala.concurrent.Future what thread pool to use for execution. Since on the JVM threads are typically pretty heavy, there's a often a lot of tuning needed at scale.

Check out the Akka docs for "dispatchers, for instance: http://doc.akka.io/docs/akka/snapshot/scala/dispatchers.html and for tuning: http://letitcrash.com/post/40755146949/tuning-dispatchers-in-akka-applications

Typically you'll set up a specific pool for "External IO" tasks and use that for all your http requests, and a separate pools for cpu and local IO bound tasks. Slick, for instance, uses it's own pool to manage all the blocking jdbc calls and expose a non-blocking API -- which would be another possible path to follow.

That said, if the plan is to move to a native NIO non-blocking back-end on the JVM side using akka http or AHC, there won't be a need for an execution context for the send() method so long as there is no further processing in RosHTTP and then the issue goes away.

Making the API platform specific would definitely not be good. If the plan is to keep using Future wrapping the blocking java api's, I'd just add the implicit ec to the external api and move on. It's not an uncommon pattern, and folks that don't care are used to importing the global one anyway. For executors where it isn't needed you could even stick an implicit val ec = Implicits.global in the companion object.

One way to come at this is to think about who your audience is. If the audience is primarily scala.js developers who happen to want the convenience of being able to cross-compile, then the performance side of the equation isn't that important and it's down to usability issues like the one I ran into with scalatest.

On the other hand, if you want this library to be the go-to scala library for making HTTP calls, at scale, on the backend, you'll definitely need a non-blocking back-end and/or some more explicit control of the thread pool in use.

Personally, I'd like this library to be usable at that scale, since I have tons of code that's currently jvm specific because I'm using play-ws. Most of it would be useful to have on the front-end, but it makes millions of calls per day on the back-end so the performance matters.

[hmil]

Thank you for this comment, very instructing.

The JVM backend has to be rewritten anyway and I don't know yet which backing API is the best candidate. Indeed a non-blocking API would be perfect because it would potentially remove some tiny differences between the js and jvm implementations. Feel free to provide your input in #25 !

[sjrd]

I agree that the initial proposed change should be done. Libraries should never assume some specific execution contexts. They should always take implicit ECs as parameters, so that using applications can have control over them.

[hmil]

All right, then I guess this goes in v2 as well!