-
-
Notifications
You must be signed in to change notification settings - Fork 90
Description
As rama is "maturing", we start to get more end user experience and also collect more feedback from other users. This didn't make the 0.2.x cut as it's an impactful change that will require some iteration to get right. But for now this is more or less how I see it.
pub trait Service<Request>: Sized + Send + Sync + 'static {
type Response: Send + 'static;
type Error: Send + Sync + 'static;
fn serve(
&self,
req: Request,
) -> impl Future<Output = Result<Self::Response, Self::Error>> + Send + '_;
fn boxed(self) -> BoxService<Request, Self::Response, Self::Error> {
BoxService {
inner: Box::new(self),
}
}
}in contrast to https://docs.rs/rama/latest/rama/trait.Service.html. The change here is that Context would be removed, if we indeed go through with this. I want to be clear this isn't a certainty yet. And this issue is mostly to start the discussion and iteration of ideas on how we might want to tackle this and anything related in 0.3.
So why would we want to get rid of Context, if at all?
This would make rama compatible with a future version of tower. And while rama might always have its own services, it might make it at least possible for us to make it possible to use tower services in rama and rama services in tower context is today used for three things.
1. State
This is the big one, but as rama starts to become more mature and I have more experience with it as an end user I start to think that we pay a high price for that static state. Sure it's nice, but it's also very limited in use. The real killer is to have it in web endpoint services, which is a very specific use case. And other case where I use it is to have static stuff for creating services downstream. For the first we might be able to make it still work but more the axum-approach where we perhaps tie that directly into the http server somehow and still allow it to be accessed as an extractor. Not sure how that would work though, would need some figuring out. The latter I am not convinced we really need static types for that. Might as well just put that stuff in extensions if needed or use a more functional approach by using global functions unique to the app to get access to this information. At the other hand the price for this state is really high even though in 80% of places we do not use it, you do always have to take it into account.
Why was state introduced?
State didn't come out of the blue and the current context is still pretty solid I think, coming out of many iterations of what rama now is. The biggest reason why it made the cut is because it made it very easy and clear to pass state through in a type-static manner, in contrast to anything like extensions which is dynamic and runtime-uncertain. In contrast to that something like State will stop your program from compiling all together if it doesn't contain something that you expect it to contain. To make it even more flexible there is the AsRef / AsMut trait pair that you can use to work with any state that can give you what you expect (e.g. some property or db handler of some kind).
A typical use case of state is web service endpoints. And in fact that is the only context in which I've actually used it so far, which is one of the reasons why I started to think more and more that perhaps we pay a very high price (as context and its state are to be handled by every single service out there, even though most only ever use extensions as the AsRef / AsMut concept is pretty limited in possibilities and also because a lot of stuff is actually really optional in nature anyway.
rama/rama-cli/src/cmd/fp/state.rs
Lines 7 to 14 in 9a9e309
| #[derive(Debug)] | |
| #[non_exhaustive] | |
| pub(super) struct State { | |
| pub(super) data_source: DataSource, | |
| pub(super) acme: ACMEData, | |
| pub(super) storage: Option<Storage>, | |
| pub(super) storage_auth: Option<String>, | |
| } |
Sometimes proxy services also use it to pass information on how to create certain egress services in the middle of a proxy flow:
pub(super) http_firewall_enabled: bool,
pub(super) transport_firewall_enabled: bool,
This however is a less strong reason as you could also instead inject these into the extensions. Or even differently expose them as public global functions. Because in the end these are all end user cases anyway, so I would think it's less dirty to expose these as global statics, especially if they are behind a clean functional API. Even for configs loaded from config files or cli args you can still expose them as static (behind functional API) by removing such variables out of the tracking and leak them as statics. A lot is possible like that and imho still a lot better than a high price to pay for such limited use case.
2. Executor
This one is mostly there so one can easily spawn graceful-aware tasks. Now you gotta know that in the past even Dns was part of the context for similar reasons. And I pulled that out of there as it was a pretty edge case thing to be in there. I think in the same way we can pull that executor out of context (or rather it's not a good enough reason to have context for this). Instead we could put a Guard in the Extensions and provide a function in rama::rt that would be somthing like fn spawn<F, E: AsRef<Extensions>>(r: E, task: F>) which would use the guard if available and otherwise just tokio::spawn. So same same, but without the need to drag a context along.
3. Extensions
This one has always itched me a bit but so far I've been ok with it. It essentially means that as soon as you enter http world you have 2 extensions collections. Instead I imagine a world where we could have just only the one in the http request. That would mean we can also make a Request type for transport layers, that contains a Stream and the Extensions. Similar to the http Request. Even better is if we can somehow use the same Extensions type as that would mean we can forward the extensions to the http layer if we cross that ISO layer. To make that happen ideally the extensions type of http is pulled out of the http crate though as otherwise we would always need to pull that crate. And other idea is of course to instead put the transport extensions as a single value in the http extensions. That would not require any changes in the http crate and be perhaps even cleaner.
If you take this all together I think there is enough in here that we can perhaps get rid of that Context and make our life a whole lot easier. https://docs.rs/tokio/latest/tokio/macro.task_local.html is another possible way to put static state, but haven't played with it yet.
Again this is for now just how I think about it, and there's still plenty of time before we would even start any work in this direction. First I still want to focus on getting 0.2 out of the door, and 1 or 2 releases after that as 0.2.1 and perhaps also still a 0.2.2. This because:
- there are still many nice features missing that I want to have added first before changing some fundamental designs that anyway aren't a real blocker for now
- but also this gives people plenty of time to chip in, and also other ecosystems to continue evolve around us (e.g. how will tower shape up by then, etc...)
This issue is mostly for now a tracker issue and a central place to chip in ideas, provide use cases for one argument or another, and to have a healthy long ongoing discussion.