[Feature] Add ability to shutdown server once it has been started

[trezm]

related to #78 and #46

[naiveai]

I see you have this tagged as "good first issue". Is there any chance I can pick it up? I'm reading through the docs & code to get a better idea of everything but this seems pretty doable to me.

[trezm]

Absolutely, that would be great! Let me know if you have any questions/pointers. A note: because thruster allows you to use a variety of different backend servers (hyper, homegrown, etc.) I'd recommend just focusing on this feature for hyper initially, then we can examine whether we want to extend it to the other built in servers or not!

Thanks for picking it up!!

[naiveai]

@trezm I apologize, I don't have a whole lot of familiarity with Rust async in the real world, and I wanted to clarify a few things.

After server.start is executed, am I right in saying that it isn't even possible to execute anything afterwards, as you said in #78? So how would anyone use a server.shutdown method, if implemented?

Secondly, as far as I understand, tokio::Runtimes are shutdown when they are dropped. So, my idea so far is to write server.shutdown or a Drop impl that drops the Runtime value created by ThrusterServer::start.... somehow. Am I on the right track?

Lastly, are there any tests/verification methods I can use to see if everything still works alright? Should I just run the examples?

Again, I apologize at how new I am to this, I don't want to put all the work on you, but I also want to make sure that I don't make any weird API changes and spend a lot of time debugging something that's never gonna work.

[trezm]

No apologies, please! This is a good first issue for a reason, it's for learning!

Okay, so the initial assumption is kind of right. In the same task, nothing else would execute after server.start. That being said, you could spawn a separate task and run it there, e.g.

// Some code here

tokio::spawn(async move {
    server::start(host, port)
});

// This code is also executed

With this in mind, what really needs to happen is the server needs to be able to watch for some sort of signal. I think the safest way to do this would be to add method to the server trait that's something along the lines of build_with_signal_channel. The default implementation of this can just ignore the channel and call build internally.

I recommend starting with the hyper server because they already implement a graceful shutdown mechanism that you can use.

We could also use futures resolving like hyper does. Honestly I'm fine either way.

Hope that helps!

[naiveai]

@trezm I hate to do this, but I use Windows and don't have administrator rights for my computer at the moment, so getting everything compiled and the tests running has been an ordeal that I wasn't able to accomplish.

I'm sorry for dropping the ball, but I've been splitting hairs over this and I don't think I can do this until I get admin rights, which probably won't be for a while.

[trezm]

Not a problem whatsoever! Thanks for your interest either way. I'm sure this conversation will help someone finish the bug in the future :)