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

decide what to do with tower #5

Closed
GlenDC opened this issue Nov 26, 2023 · 1 comment
Closed

decide what to do with tower #5

GlenDC opened this issue Nov 26, 2023 · 1 comment
Labels
help wanted Extra attention is needed needs input question Further information is requested

Comments

@GlenDC
Copy link
Member

GlenDC commented Nov 26, 2023

That rama is to be built in the "tower" spirit is clear. However at the moment we make use of https://github.com/plabayo/tower-async rather then the original https://github.com/tower-rs/tower. Reason being that only with a lot of boxing one can make use of async functions within a Tower Service.

For now tower-async fulfils that needs. However:

On top of that it means we have to put in the work to maintain an entire alternative tower ecosystem ourselves.

tower-rs/tower#753 was opened to help decide that.

So far it seems that the impl_trait_in_assoc_type usage would be the way to go and would allow us to make use of tower as-is, and refer to futures. Where we need async opaque types we can make use of type Future = impl Future<...>. However what is less clear is if they also want to go for &self instead of &mut selfand whether they want to get rid ofpoll_ready` in the default contract. Which would still be desired.

Tracking issue for assoc type: rust-lang/rust#63063

As there is a lot of unclarity here, and as tower-async does work just fine for now and we already spent a lot of time on it, for now we can just keep it as-is. Once there is more clarity in regards to the direction of tower, and once we have time for it, we can look back it it. Anyone who wishes can also provide feedback here in regards to tower usage within rama and the direction we might want to take.

@GlenDC GlenDC added help wanted Extra attention is needed question Further information is requested needs input labels Nov 26, 2023
@GlenDC
Copy link
Member Author

GlenDC commented Dec 23, 2023

After consideration, taking into account feedback in tower-rs/tower#753 as well as reading through https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html and linked resources, it seems clear that the async story is not yet ready for tower+tokio like systems. On top of that we clearly run into:

  • time issues as maitaining this entirely on my own takes a lot of time;
  • it also takes time away from actually being able to contribute to tower itself;
  • limitations, where stuff like boxing for rama purposes cannot be supported unless we make the entire tower-async Send enforced... (see reintroduce boxed service tower-async#12)

So instead... I think, we'll cut our losses, consider what we learned from it... and,.... move back to tower itself.
This does mean we might need to box for now, but it does also mean we can at least move back to Rust stable, and have less work in it.

On top of that I start to doubt that it really makes sense to try to wrap everything. For the runtime it does make sense, but for tower I do think it's a bit much... Instead I start to like more and more the way Axum handles it.

Lastly, if we go for regular tower services it will also play a lot nicer with hyper as-is.

It is also probable that the combination of being able to run on rust stable + using the regular tower ecosystem, would allow others to adopt rama a lot easier.

GlenDC pushed a commit that referenced this issue Dec 25, 2023
@GlenDC GlenDC closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed needs input question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant