API design to get hold of a Tokio handle within a handler

[ChristophWurst]

As discussed on Gitter with @bradleybeddoes, I'm missing the possibility to send new HTTP requests inside Gotham handlers because currently it's not possible to get a reference to the underlying Tokio event loop.

@bradleybeddoes pointed me to

gotham/src/handler/mod.rs

Line 151 in 953c39b

fn call(&self, req: Self::Request) -> Self::Future {

where other framework state variables are initialized. Unfortunately, I don't see a way of accessing the tokio handle in that function. As a matter of fact the whole Gotham codebase is pretty much unaware of the whole Tokio stuff because the way Gotham apps seem to be started is that the handler is passed to Hyper's server API. Hyper will either create the Tokio core internally, or let you use an existing one (since #66).

An alternative to having Gotham put the handle reference into the state object would be to use a middleware for that: https://github.com/ChristophWurst/gotham-middleware-tokio. You can find an example usage here: https://github.com/ChristophWurst/gotham-middleware-tokio/blob/master/examples/hyper_client.rs#L22

There seem to be a few problems though. I don't know whether we should use a Handle, a Remote or let the developer choose. Handle cannot be shared across threads. However, Hyper expects a Handle rather than a Remote for its client, making it a bit cumbersome to use. The remote object does have method to get a handle, although it's not guaranteed to always get one.

This is somewhat related to hyperium/hyper#1002

@bradleybeddoes @smangelsdorf thoughts on how to integrate this in Gotham?

[bradleybeddoes]

I'm not sure what @smangelsdorf had in mind but I was thinking of something along the lines of passing a handle reference to

gotham/src/handler/mod.rs

Line 123 in 953c39b

pub fn new(t: T) -> NewHandlerService<T> {

and then being able to use that within

gotham/src/handler/mod.rs

Line 151 in 953c39b

fn call(&self, req: Self::Request) -> Self::Future {

to push it into state.

I may be missing some importance nuance here as I've only very briefly considered this, happy to keep kicking this around if I've overlooked something important.

If we do change

gotham/src/handler/mod.rs

Line 123 in 953c39b

pub fn new(t: T) -> NewHandlerService<T> {

then #70 will need to consider the impact.

[rcaught]

Related: #69 will need access to the event loop as well.

[smangelsdorf]

I don't know whether we should use a Handle, a Remote or let the developer choose. Handle cannot be shared across threads. However, Hyper expects a Handle rather than a Remote for its client, making it a bit cumbersome to use.

Tokio was discussing a change which merges both these types into a Handle which is also usable in the way Remote is currently used. If that's still their plan (which seems likely), sticking with what Hyper wants will ensure we automatically get the benefit of this refactor when it happens.

[smangelsdorf]

@bradleybeddoes:

[…] passing a handle reference to

gotham/src/handler/mod.rs

Line 123 in 953c39b

pub fn new(t: T) -> NewHandlerService<T> {

and then being able to use that within

gotham/src/handler/mod.rs

Line 151 in 953c39b

fn call(&self, req: Self::Request) -> Self::Future {

to push it into state

One complicating factor here is that we're creating a NewHandlerService and then cloning it for each connection:

gotham/src/clone_sockets.rs

Lines 21 to 35 in 953c39b

	let service = NewHandlerService::new(new_handler);
	info!(
	target: "gotham::start",
	" Gotham listening on http://{} with {} threads",
	addr,
	threads,
	);
	for _ in 0..threads - 1 {
	let listener = listener.try_clone().expect("unable to clone TCP listener");
	let protocol = protocol.clone();
	let service = service.clone();
	thread::spawn(move || serve(listener, &addr, &protocol, &service));
	}

The structure here might have to change a bit, to have NewHandlerService::new take an Arc<T> and a Handle. The local serve function could then be changed to take the Arc<T> and have it create the NewHandlerService.

With this small refactor I think your suggestion will work well. 👍

[smangelsdorf]

then #70 will need to consider the impact.

I can't see where there would be any impact on #70. Did you have something specific in mind?

[bradleybeddoes]

I can't see where there would be any impact on #70. Did you have something specific in mind?

Apologies I mis-spoke on this one, please ignore that comment.

[bradleybeddoes]

🎉 😂 💫 💌 📣