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

Consider removing futures 0.2 feature flag in favor of an external compatibility layer #312

Closed
carllerche opened this issue Apr 10, 2018 · 6 comments

Comments

@carllerche
Copy link
Member

Futures 0.2 support was added as feature flag. However, the integration is not complete and not functional (#251).

The recommendation from the futures team is to not update libraries fully to 0.2.

Hyper has already reverted futures 0.2 support in favor of providing an external compatibility layer. Ideally, the compatibility layer would be provided by the futures team, but I do not believe that there are plans to do so.

If Tokio removed the feature flag in favor of this compatibility layer, it would simplify the existing code base.

cc/ @aturon @srijs @seanmonstar

@aturon
Copy link
Contributor

aturon commented Apr 10, 2018

I'm away on vacation, but to clarify, my hope was that the foundational libraries (Tokio and Hyper) would offer direct compatibility with 0.1 and 0.2, as in the patches I landed previously, to make it possible for people to experiment directly with the 0.2 APIs. Removing these patches is going to make an eventual transition to 0.3 harder, and I don't really understand the motivation for doing so.

As far as I'm aware, the use-after-free bug remains ready to be fixed as soon as the longstanding PR for it is merged.

@seanmonstar
Copy link
Member

@aturon the PR is not mergeable, since there was a large refactor of the thread pool since it being submitted, and I haven't had the time to update it.

As for support in hyper, I had to revert it since the futures 0.2 announcement says it's not going to be stable for long. The good news is that the futures-compat crate has been updated a bit this week, and is actually in a place where someone not wanting stability could use it and experiment, as requested.

@carllerche
Copy link
Member Author

@aturon as Sean mentioned the PR is not in a mergeable state.

It does not seem like anyone has the resources to maintain good support as a feature flag right now. Also, as Tokio iterates the integration lags (see timer and threadpool).

A compatibility layer covers any new changes and Tokio + Hyper in one go.

But, stepping back for a moment. What are the goals? If the goals are to enable end users to be able to experiment with futures 0.2 in their applications, a compat layer will be the quickest and lower friction way to do this.

If the goal is to do more in depth vetting of the new model, then we need to invest the necessary resources to do it right.

@aturon
Copy link
Contributor

aturon commented Apr 11, 2018

@carllerche Understood re: maintenance -- the Rust All Hands in Berlin slowed down my response time considerably. Still, I'd prefer to retain the deepest vetting and most direct experience we can, and will work to resource it appropriately.

@carllerche
Copy link
Member Author

@aturon I am not going to take any immediate actions, so we can discuss again when you are back from PTO.

@aturon
Copy link
Contributor

aturon commented Apr 18, 2018

@carllerche I'm not sure if you've seen the proposed rollout here, but I expect to have a working alpha version of futures 0.3 in the near future (next couple of weeks); you can see the initial work here.

As such, again I'd very much appreciate it if we can leave this code intact, so that I don't have to redo this work for 0.3 feature-flag integration.

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

No branches or pull requests

3 participants