Add Tokio support

[afdw]

Similar to #24, but without duplicated functionality and more carefully thought out, although there might still be mistakes. This one does not change existing code at all, only adds new module named tokio_support. It works by calling functions from the root of the crate in thread where blocking is acceptable and communicates with this thread using channels, thus implementing Read and Write in terms of AsyncRead and AsyncWrite. This allows to use the library in applications that are based on Tokio, mainly in client/servers. A usage example named async_uncompress_service is provided, along with it sync version uncompress_service for comparison.

[coveralls]

Pull Request Test Coverage Report for Build 231501058

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 14 of 22 (63.64%) changed or added relevant lines in 2 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+19.8%) to 75.776%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/error.rs	0	2	0.0%
src/async_support.rs	14	20	70.0%

Files with Coverage Reduction	New Missed Lines	%
src/error.rs	1	44.0%

Totals
Change from base Build 231483955:	19.8%
Covered Lines:	122
Relevant Lines:	161

---

💛 - Coveralls

[otavio]

@afdw first I'd like to thank you for this great work.

I think this goes in the right direction and solves the concerns we had regarding code duplication. There are a couple of things I'd like to ask if we could rework on this PR prior merging it.

First is to spinoff the code which is independent and which we could merge right away. Here I am referring to the uncompress_service which could be added in a specific PR for it.

Next, I'd like to explore the possibility of making this independent of tokio and base it on top of futures-io and let the runtime choice for the app. What do you think about it?

[afdw]

First is to spinoff the code which is independent and which we could merge right away. Here I am referring to the uncompress_service which could be added in a specific PR for it.

#28

Next, I'd like to explore the possibility of making this independent of tokio and base it on top of futures-io and let the runtime choice for the app. What do you think about it?

There is a couple of problems with that:

1. The AsyncRead and AsyncWrite traits in tokio and futures-rs are incompatible (they are not just reexports of one another, they are just completely separate definitions).
2. There is no standardized way of executing blocking code (no generic tokio::task::spawn_blocking).

This is a good idea, I want it be independent of Tokio too, but at the current stage of the async infrastructure I don't think that's possible. Or do you have any ideas of how to solve the problems mentioned above?

[otavio]

1. The `AsyncRead` and `AsyncWrite` traits in `tokio` and `futures-rs` are incompatible (they are not just reexports of one another, they are just completely separate definitions).

Sure and tokio users can still use the futures-rs ones if use tokio-util's compat method.

2. There is no standardized way of executing blocking code (no generic `tokio::task::spawn_blocking`).

For this, we can use blocking. What do you think?

[afdw]

1. The `AsyncRead` and `AsyncWrite` traits in `tokio` and `futures-rs` are incompatible (they are not just reexports of one another, they are just completely separate definitions).

Sure and tokio users can still use the futures-rs ones if use tokio-util's compat method.

Thanks, I have not seen that. Yes, this is indeed possible.

2. There is no standardized way of executing blocking code (no generic `tokio::task::spawn_blocking`).

For this, we can use blocking. What do you think?

That is one of the options. The problem is that a separate (from Tokio's one) thread pool will be used, which is may or may not be desirable. What do you think can be done with that? I think offering 2 modules (tokio_support and futures_support, each offering the 4 library functions) might be the easiest solution.

[otavio]

What about offering a single async module which has the async-std and tokio implementations and user choose which one to use with a feature for each one?

This could basically use the runtime specific spawn_blocking.

[afdw]

The problem with using a single module is that than it will not be possible to, for example, run cargo test --all-features, as both both support modes could not be meaningfully enabled at once.

[otavio]

The problem with using a single module is that than it will not be possible to, for example, run cargo test --all-features, as both both support modes could not be meaningfully enabled at once.

Indeed.

So, in that case, we need to decide. I'd be Ok in using blocking and would be fine in having two implementations. What I think we ought to consider is if it is worth adding two implementations to save a thread pool.

What is your preference?

[afdw]

I do not think that having 2 implementations is problematic, they are just very thin and simple wrappers. So I have implemented them. But reducing repetitiveness would be good. The problem is that they have many things in common, but have some subtle differences, so generating them with a macro is not that straightforward, but actually could still be done.

[otavio]

I think we need to document the features so people can make the right choices. Also, can you squash the commits and add the examples as later commits so we isolate the changes in logical units?

[otavio]

Also, please rebase the changes.

[afdw]

I think we need to document the features so people can make the right choices.

There is some documentation already:

What do you think needs additional clarifications?

Also, can you squash the commits and add the examples as later commits so we isolate the changes in logical units?
Also, please rebase the changes.

I will tidy up the commit history as soon as everything else would be resolved.

[Jonathas-Conceicao]

I would also like to change the flags and module names of tokio_support and futures_support to just tokio and futures, I think this would be just as much meaningful. As for the async_support I'm fine with it since async is a keyworld and can't be a module name.

[afdw]

I would also like to change the flags and module names of tokio_support and futures_support to just tokio and futures, I think this would be just as much meaningful. As for the async_support I'm fine with it since async is a keyworld and can't be a module name.

The problem is that it is impossible to have a dependency and a feature with the same name.

[otavio]

I would also like to change the flags and module names of tokio_support and futures_support to just tokio and futures, I think this would be just as much meaningful. As for the async_support I'm fine with it since async is a keyworld and can't be a module name.

The problem is that it is impossible to have a dependency and a feature with the same name.

Yes and for this, we can map the package internally, so we can expose the feature name.

[afdw]

I would also like to change the flags and module names of tokio_support and futures_support to just tokio and futures, I think this would be just as much meaningful. As for the async_support I'm fine with it since async is a keyworld and can't be a module name.

The problem is that it is impossible to have a dependency and a feature with the same name.

Yes and for this, we can map the package internally, so we can expose the feature name.

I am not really sure that would be better. This maybe confusing, as someone may think this are reexports of tokio and futures. Also, I kind of like the symmetry that current names have.

Do you have any alternative ideas on how to make the module naming better?

[otavio]

@afdw it is a fair justification. I was discussing with @Jonathas-Conceicao and we agreed it is a good way to go.

Could you rebase the tree and prepare the commits for review/merging?

[afdw]

OK, done?

[otavio]

@afdw just made 0.7.0 release. Thanks for all your help :-) awesome!