Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow launching on a custom Tokio runtime #1097

Closed
wants to merge 4 commits into from
Closed

Allow launching on a custom Tokio runtime #1097

wants to merge 4 commits into from

Conversation

jhpratt
Copy link
Contributor

@jhpratt jhpratt commented Aug 18, 2019

Currently only on a Tokio runtime for reasons described in #1068. In the future, it would naturally be ideal to allow launching on any runtime; such a change would necessitate larger code changes to provide for a common API.

Resolves #1068.

@jebrosen
Copy link
Collaborator

launch_on should take &Runtime or possibly TaskExecutor; moving Runtime by value probably precludes most of the useful reasons to do this.

block_on shouldn't be called inside launch_on, for a similar reason. I suspect launch_on will need to return some kind of future or spawn handle.

Ideally we could have a method that took only self and returned a future that could be spawned by the user on any tokio runtime, but we currently need the executor "ahead of time" in order to pass it to hyper.

@jhpratt
Copy link
Contributor Author

jhpratt commented Aug 21, 2019

Ah, I originally assumed this was just to be able to customize the runtime, not the ability to launch on a runtime that may already exist and does other things. That's certainly a different situation.

I've opted to accept a &Runtime over an &Executor, as that will be helpful when #1069 is implemented.

core/lib/src/rocket.rs Outdated Show resolved Hide resolved
core/lib/src/rocket.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@jebrosen jebrosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to merge after these minor adjustments so that #1079 is combined with these changes.

I think both functions will change further, but this is a great starting point to play with the functionality.

core/lib/src/rocket.rs Show resolved Hide resolved
core/lib/src/rocket.rs Outdated Show resolved Hide resolved
core/lib/src/rocket.rs Show resolved Hide resolved
@jebrosen
Copy link
Collaborator

Merged as 70ca157.

@jebrosen jebrosen closed this Aug 24, 2019
@jebrosen jebrosen added the pr: merged This pull request was merged manually. label Aug 24, 2019
@jhpratt jhpratt deleted the custom-runtime branch August 24, 2019 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: merged This pull request was merged manually.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants