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

Move tokio::Runtime into ClientFactoryInternal #107

Closed
tkaitchuck opened this issue May 29, 2020 · 0 comments · Fixed by #111
Closed

Move tokio::Runtime into ClientFactoryInternal #107

tkaitchuck opened this issue May 29, 2020 · 0 comments · Fixed by #111
Assignees

Comments

@tkaitchuck
Copy link
Member

Problem description
Right now both the connections to the server and the controller require a Tokio runtime, but it is also needed to spawn background tasks such as in EventStreamWriter. This has had the unfortunate side effect of making methods that don't need to be async into async methods just so they can call spawn: #104 (comment)
Additionally it means that each test must create a Runtime, which leads to redundant setup logic, which we have avoided by combining multiple tests into one.

Suggestions for an improvement
Instead the Runtime should be created and held by ClientFactoryInternal and provide an accessor so that any code in the client can spawn tasks without needing to be async or worry about the context in which it is called. At the same time this should simplify our unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants