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

RFC: isolating a stable core #162

Open
aturon opened this issue Apr 10, 2019 · 30 comments
Open

RFC: isolating a stable core #162

aturon opened this issue Apr 10, 2019 · 30 comments
Labels
design Open design question

Comments

@aturon
Copy link
Collaborator

aturon commented Apr 10, 2019

Tide is designed to be a highly modular, extensible framework, and #156 pushes us further in that direction.

There are a set of core concepts and interfaces:

  • The core routing system (App and Route)
  • The Endpoint interface, including Context
  • The Middleware interface

Beyond that, we have a number of implementations and extensions:

  • Various "extractors", which are now implemented as extension traits that work on Context. See forms for example.

  • Middleware implementations, such as logging.

  • Response builders, like this one.

Tide's current design allow for a strong decoupling between the core interfaces and these various extensions, much like Flask. This raises some questions about how to approach development, as well as the UX for Tide.

Proposal: follow the Flask model (for now)

Since Tide is explicitly intended to foster re-usable, modular components that can work with other frameworks, it makes a lot of sense to take full advantage of the decoupling above. That would follow in Flask's footsteps, where there's an extensive ecosystem of independent extensions to the tiny core framework.

I propose that we:

  • Limit the tide crate itself to the core interfaces and APIs, and perhaps some extremely common extractors (the ones that are currently directly provided on Context, like body_json.

  • Move all other extractors, middleware, and response builders into small, separate crates -- and generally encourage a spirit of experimentation with lots of small crates trying out competing approaches.

  • Provide a unified "guide to Tide's ecosystem" as an entry point to all these extensions. This could be as simple as a list of links to crate docs.

Eventually, as we gain more experience, I imagine that some of these extensions will become de facto standard, and we might want to consider moving them into core. Likewise, I expect we'll want some "bundling" crates that set up a middleware stack for you, to make it very easy to get going with Tide. But these things can come later.

This approach would make it feasible to get the Tide core to 1.0 status relatively quickly, with the expectation that the APIs would remain stable for a while (let's say six months at least). Meanwhile, the various middlware crates etc can be versioned independently, freeing them to make breaking changes without any coordination.

Open questions

One open question: if we do take this direction, do we want separate repos or just separate crates? I worry a bit that if we get too aggressive about separating repos right now, it will be hard to follow what's happening. So I personally lean toward at least starting by having the other crates live within the tide repo, with a shared issue tracker, until we start hitting scaling problems.

wdyt?

@aturon aturon added the design Open design question label Apr 10, 2019
@aturon
Copy link
Collaborator Author

aturon commented Apr 10, 2019

Related to #73 and #159 as well, which both touch on "middlware stacks" (and how to provide a default one).

@aturon
Copy link
Collaborator Author

aturon commented Apr 10, 2019

An important question here: as the dust settles from #156, can we find ways to make it easier to write middleware that works across multiple frameworks? E.g., Actix has its own notion of middleware that's not too far off from Tide's. But it's not obvious that the differences can be easily bridged. Maybe it's possible to define a "least common denominator" interface that offers generic wrappers for various frameworks.

@rolftimmermans
Copy link

rolftimmermans commented Apr 11, 2019

Having a universal middleware concept in the Rust web-ecosystem is something I look forward to. In the Ruby community, for example, every framework builds on Rack middleware. I see this as the sole factor that allows for (some) code reuse across frameworks.

My questions would be:

  • If Actix middleware is different, why is it different? Does Tide middleware offer the same functionality, and could they be mapped to each other? Is it technically possible to move to a common middleware in the future?
  • Can we come up with a definition of middleware that is not dependent on any implementation details of the framework? For example I see in the current Tide middleware definition that it depends on the DynEndpoint type. Ideally the endpoint itself should also be expressed as a middleware so that no knowledge about endpoints is needed in order to define middleware.
  • Maybe the Middleware type and related code needs to be the "core", kind of in the same way that Rust stdlib provides futures, but you need other projects to actually work with them.

Edit (from @aturon): I've created a dedicated issue for further discussion along these lines.

@yoshuawuyts
Copy link
Member

if we do take this direction, do we want separate repos or just separate crates?

I think we can probably follow in gloo's footsteps here, and create a whole lot of subcrates + a skeleton to RFC a design for a crate. This would allow us to build out a fair share of middleware without incurring too much of the maintenance overhead.

Also in particular their approaches to versioning and release scope might be interesting to read up on.

@prasannavl
Copy link
Contributor

prasannavl commented Apr 11, 2019

Glad to see this direction!

Couple of quick thoughts:

  • Agreed 100% on keeping tide as core and interfaces only.

  • Middlewares are best kept in a separate tide-blessed crate. It shouldn't matter much if they're a part of the same repo, or a different one. Versioning is the bit that becomes a little more tedious, but shouldn't be too much of a problem.

  • That being said, I do think a separate repo will encourage and make things easier if cross-framework middleware is the goal, and on this note, make versioning easier rather than being tied too much into the tide repos.

  • Aligning with the above notes: I've already published a crate surf keeping up with the tide, that I built for our uses since we're head strong to take tide into production. Currently, it has the following:

    • RequestLogger: Simple logger that doesn't take any dependencies other than the log crate. Supports timing as well. I eventually plan to providing with custom formatting and more customizations as well.

      use surf::middlewares;
      app.middleware(middlewares::log::RequestLogger::new().timed());
    • CorsBlanket: A blanket middleware for Cors.

      use surf::middlewares;
      app.middleware(middlewares::cors::CorsBlanket::new()
           .origin(HeaderValue::from_str("https://surf-with-the-tide")));
    • And likely that requestid, auth handlers, etc, might make it in there soon.

    • I intend to keep master on tide's master branch.

    • Still needs tests, and docs, but I'd be happy to merge these back into tide, depending on what you decide with the above :)

@bIgBV
Copy link
Contributor

bIgBV commented Apr 11, 2019

@prasannavl re: logger, I think following the footsteps of actix would be a good idea. Simply use the macros from the log crate and users of the framework can use a crate which implements the Logger interface. This could be something like env_logger, slog or even tokio-trace. I think that this is the most flexible approach.

@prasannavl
Copy link
Contributor

@bIgBV - That's exactly what the RequestLogger in surf does. Plus a few other goodies like timing, and more planned like custom formatter, all on top of whatever logger is being set up by the log crate by the application.

@aturon
Copy link
Collaborator Author

aturon commented Apr 11, 2019

@rolftimmermans great comment -- I liked it so much I've moved it to its own issue, as I think cross-framework middleware deserves its own focused discussion.

(That said, as folks have noted in this thread, there's some potential impact on where middleware lives and how it's named etc, depending on whether it's Tide-specific)

@mmrath
Copy link
Contributor

mmrath commented Apr 13, 2019

While there are advantages of keeping core small and then building app with pluggable ecosystem crates, I am more in favour of a meta crate that pulls together necessary bits to get me started. The basic things should all be provided by the crate.
One approach to extract core apis to a crate(which is currently tide) named tide-api and then other crates depend of tide-api. tide can be the meta crates with enough batteries to get started without pulling 10 others crates directly.
It is a bit of overhead if I have to choose crates for very basic things.

@prasannavl
Copy link
Contributor

I'd rather prefer it handled outside of tide for the time being, as the tide story first becomes production ready and has a little bit of time to stabilise. This will put tide in a much better place on how best to do this. I've been examining how to get this up and running in surf myself.

I've been thinking of helper patterns similar to surf::app::new or surf::app::with_common, or if it should be called AppBuilder with a builder pattern -> both of which basically return a tide::App with batteries (common middleware, defaults, etc). Now, while it's just me with surf and have no concrete plans on it's future, I think in general it's a good idea for tide to encourage being experimented this way to gather a little more on it's usage patterns before rushing into something like tide-api.

@yoshuawuyts yoshuawuyts mentioned this issue Apr 15, 2019
8 tasks
@secretfader
Copy link

Discussions after today's meeting seemed to center around the need to separate Tide core from contributed extensions that, while they may be common, aren't needed for every application that depends on Tide.

Based on an idea from @bIgBV, I propose the following:

  1. All non-essential extensions live in a tide-contrib repo, created under this organization, which functions as a multi-crate workspace.
  2. Common macros that are needed by extensions in that repo can be placed in a shared crate. (If we need to bundle multiple crates into commonly used bundles, that's an option, too.)

#175 is a solid example of a feature that essentially all Tide apps will need, while session handling is a concern that's slightly out of scope for core, in my opinion.

@aturon
Copy link
Collaborator Author

aturon commented Apr 18, 2019

We had some extensive discussion on Discord, which led to the following.

Goals

  • make it easy for folks to land new, experimental extensions
  • make it easy to make breaking changes to experimental parts of the framework, without disrupting the more stable parts
  • provide a nice, batteries-included entry point to the framework, so that it's not too much work to get set up to handle common needs
  • be careful not to provide too many abstraction layers if we want people to write http services
  • encourage cross-framework components (middleware, extractors, etc)

Proposal

(from @yoshuawuyts)

  • Single repo, tide, with a workspace of multiple crates
  • The tide crate contains the core APIs and the "out of the box" experience we're currently willing to commit to
    • Evolves relatively slowly, tries to avoid breaking changes
  • Lots of additional tide-foo crates defining middleware or other extensions
    • Separately versioned, can make breaking changes easily
    • As crates mature, if they are "core" enough they can be proposed for graduation into tide proper
  • We continuously look for opportunities to factor out code into cross-framework crates that live directly under rustasync in their own repo

@yoshuawuyts
Copy link
Member

@rustasync/maintainers would anyone be interested in picking up this issue for this sprint? It seems like this would likely make the biggest structural improvement to our process, and free us up to do more experiments, and iterate faster ✨

@wafflespeanut
Copy link
Contributor

I can pick this up!

@secretfader
Copy link

You're welcome to it, @wafflespeanut. I'll be on the lookout for a PR. Thanks for helping out!

@prasannavl
Copy link
Contributor

prasannavl commented May 13, 2019

@wafflespeanut - Awesome! Splitting this up into smaller units to make this more manageable into smaller PRs with the current middlewares

Isolating Core - Stage 1

Precursor

(Non breaking, foundation bits for the entire organisational structure):


Core Isolation

(Initial run, we could even do most of the isolation in a non-breaking way - hopefully even everything, but fingers crossed and assume breaking for now)

  • tide-core - Needed so that the other tide- can depend on this, and we can pull everything together into tide, or we end up with either having the whole thing in core, or have it separate with no transitive path where crates can graduate into tide in a non-breaking way (Since that will end in cyclic dependencies) Split tide into smaller crates #220
  • tide - Tide core + graduating dependencies.
  • tide-slog (What's currently called the RootLogger)
  • tide-cookies Split tide into smaller crates #220
  • tide-headers / tide-default-headers (Bike shedding needed for names?)

Before next release

Middlewares

Others

@secretfader
Copy link

We can bikeshed on crate names later. I don't think that's the most critical portion of this discussion. Ideally, the PR addressing this RFC should initially focus on the following:

Splitting Tide core into smaller crates:

Before we move any new code into this repo (from Surf or elsewhere). It makes sense to document the expected outcome. Once the split between Tide core and middleware is complete, I expect the source tree will have two primary folders in the root: tide/, which will contain the core framework implementation, and contrib/, which will contain middlewares and context extensions that aren't needed by every application, each in it's own crate (but organized in such a way that we can roll-up common dependencies into convenience crates).

I'd also like to see dependencies for example code moved out of Tide's dependency list. However, I'm aware that some dependencies are shared between examples (serde, for one), which may complicate this suggestion.

Which extensions and middleware become their own crates?

I think @prasannavl's comment above is a good place to start. Cookies can be split into tide-cookies, and Tide's existing logging functionality can become tide-slog. Tide's form extraction should probably be moved to tide-form. Past that point, I'm not totally sure if the middleware that adds default headers is worth extracting yet. Most HTTP applications will need to manipulate headers, which is a fairly strong case for keeping it in core (and should probably be the standard that decides where all components reside).

The above text encapsulates minimal changes that I think would satisfy the RFC. We can decide on the naming conventions of new crates once the bulk of this work is completed.

@prasannavl
Copy link
Contributor

prasannavl commented May 13, 2019

@secretfader - I think we've already discussed this, and there's no special contrib as such for now. Just lots to tide- middlewares for now. (#162 (comment))

We can move whatever needs to be in the core later. For now, I think we just split off and hit the ground running marking this as a ground zero of sorts, with nothing in the core at the moment. Then, we can decide what moves to core in whatever evolved form after it has had time to stabilise.

Also, I don't really see the problem with serde, however if it does indeed get into a problem splitting it into an example, then I'd label that a bug -- that needs to be fixed.

Making the PR that does 1 now. Hopefully @wafflespeanut can chime in on what he'd like to pick up form there! :)

@secretfader
Copy link

@prasannavl I think you misunderstood my comment. I'm not suggesting we create a crate for commonly used middlewares at this stage. However, think it would be helpful to place any resulting crates into their own folder, contrib/, rather than tossing all extracted code into the repo root.

@prasannavl
Copy link
Contributor

prasannavl commented May 13, 2019

Tokio, futures all of them follow this model of doing it in the root currently. Besides, everything other than tide itself is going to be a folder with middleware, so I don't see the point of the indirection at this stage. However, if you feel strongly about the organisation structure, perhaps we should arrive at a consensus there.

(Note: This could totally be a different scenario when tide's mature and have so many that tide-contrib [ideally a repo] is needed for community extensions not directly supported by the tide maintainers)

@prasannavl
Copy link
Contributor

@yoshuawuyts - I can co-ordinate this with others interested. Have done some of the foundation bits as well. I know you're busy on runtime this sprint, but would be great if you could just quickly run through the game plan here to make sure it all aligns well: #162 (comment)

@yoshuawuyts
Copy link
Member

@prasannavl

  • tide-core - Needed so that the other tide- can depend on this, and we can pull everything together into tide, or we end up with either having the whole thing in core, or have it separate with no transitive path where crates can graduate into tide in a non-breaking way (Since that will end in cyclic dependencies) Split tide into smaller crates #220
  • tide - Tide core + graduating dependencies.

I'm thinking the distinction of tide-core and tide isn't that important. Something we've tried out in rustasync/runtime#22 is the addition of conditional dependencies based on targets.

I could imagine there would be a way to setup feature flags for Tide that by default you get what you need. And then there's a feature that when enabled removes all dependencies, and you can configure everything yourself.

I suspect that adding in this mechanism would be an extra 100 or so lines, but would allow us to streamline things reasonably (:


Separately from that runtime's dir structure is pretty nice; we use src for the runtime crate, so we never have to traverse through the nested runtime paths: runtime/runtime/src/lib.rs becomes runtime/src/lib.rs. But don't feel too strongly about this (:

@Nemo157
Copy link
Contributor

Nemo157 commented May 15, 2019

One of the primary usecases of a separate -core crate is to allow semver breaking updates of the non--core crates if those are primarily used as private dependencies of libraries/applications. As long as all public API of downstream crates use only types from the -core crate they can interoperate even if they're using different versions of utilities internally. (This is why futures is split the way it is).

If you expect to never have a semver breaking update of anything that gets graduated into tide then the split is unnecessary, and it's probably possible to do a backwards compatible split in the future if needed as long as all non-core parts are feature-gated from the beginning (similar to how log has provided compatibility between 0.3 and 0.4).

@prasannavl
Copy link
Contributor

prasannavl commented May 15, 2019

I'm thinking the distinction of tide-core and tide isn't that important. Something we've tried out in rustasync/runtime#22 is the addition of conditional dependencies based on targets.

@yoshuawuyts I think @Nemo157 put across what I had in mind beautifully with the future backed experience. It'd be simpler for the middleware crates to iterate and grow easily without tying into the whole batteries included tide. Target based dependencies fit naturally for the runtime world - but I think the futures approach is a much better fit for tide.

That being said, if we don't have consensus - one way to delay this is to keep tide-core a private dependency for now that's only taken on by tide- in this repo. For the rest of the world, it's exactly as tide functions now.

Also, while this is easily understood, will just iterate for the sake of completion - tide-core ideally gets only absolute essentials that are cherry-picked from tide proper. Since we have essential middlewares in the same repo as well, we should be able to get a use-case driven guidance on that instead of exposing things like App, Server etc from the get go.


Separately from that runtime's dir structure is pretty nice; we use src for the runtime crate, so we never have to traverse through the nested runtime paths: runtime/runtime/src/lib.rs becomes runtime/src/lib.rs. But don't feel too strongly about this (:

Yes. I liked the structure when I saw in it gloo as well. But decided to adopt futures and tokio style dir for the moment - But I don't really have any preference regarding the runtime dir. Should easy enough to switch out as well should you prefer to have it that way.

@yoshuawuyts
Copy link
Member

If you expect to never have a semver breaking update of anything that gets graduated into tide then the split is unnecessary

@Nemo157 yeah, that was the exact idea I had in mind. Things only graduate once we're ready to commit them feeling like the external API is ready to be tied to Tide's stability model.

But given that they'd be used internally to Tide a lot of the times, that constraint probably feels even more flexible.


@prasannavl Up until now I was under the impression we'd achieved consensus on having a single top-level tide, without the need for -core crate when we consolidated #162 (comment). Perhaps there was a miscommunication?

Tide vs Tide-Core

I feel we've seen a lot of experimentation around core vs ext libs, (e.g. futures-core, tokio-core), and generally it seems people mostly care about not having to worry about what to use. I suspect that a distinction between tide-core and tide doesn't buy us any extra semver mobility, because almost everyone would opt to use tide directly.

Similarly we can probably learn from syn, where a lot of the features are hidden behind the "full" flag, which is also what most people want to be using. So instead of making the default experience opt-in, we make the stripped down experience the deviating choice.

# full
[dependencies]
tide = "1.3.2"

# minimal
[dependencies]
tide = { version = "1.3.2", features = [ "bare" ] }

Graduating dependencies

But in order to encourage productivity, we probably want to provide people with ways of trying out new additions, after we feel that things might be good, but before we commit to stabilizing.

# stable
[dependencies]
tide = "1.3.2"

# experimental
[dependencies]
tide = { version = "1.3.2", features = [ "cookies-experiment" ] }

I'm not 100% convinced how important this last bit is, but it might be something to consider if we want to enable some nicer workflows. It's not something we have to decide now, but can revisit as we start thinking of graduating dependencies to stable.

@spacekookie
Copy link

Just wanna chime in and say that I very much agree with @yoshuawuyts idea of having the default experience be as inclusive as possible.

What always annoyed me so far with a lot of web service development in Rust is the shear number of imports and external crates required to explicitly mention.

Having tide as one dependency with no additional features doing what you probably want will remove a lot of barriers.

Similarly, I feel like we should be sparse with feature flags. Having a generic unstable flag is something I've seen on some crates and should generally give people enough space to play around with APIs before committing to them, without again making a user include 12 different feature flags.

@prasannavl
Copy link
Contributor

prasannavl commented May 15, 2019

Before anything, let me clarify this: Everything that's being discussed above, from a user perspective is a single tide dependency only. User of tide just use tide. This is of relevance only to tide extension, development and middleware authors.

@prasannavl Up until now I was under the impression we'd achieved consensus on having a single top-level tide, without the need for -core crate when we consolidated #162 (comment). Perhaps there was a miscommunication?

Yes. This is true. It appears I'm the one who didn't get the nuances of the details correctly. Sorry about that if it was my bad - I naturally ended up with the split, because that's the only way to really have tide- repos for the middleware currently in the tide repo (which again to my understanding was the decision taken). So, I was under the impression that was only a question of what was exposed publicly.

I'll put things in context here:

Tide only

  • We can't have middleware and tide managed independently while being exposed from the tide crate, as that involves a cyclic dependency (unless we resort to more feature based dependency shenanigans here which could not be the most ergonomic)
  • tide-cookies will have go into tide. Can't be a separate crate - I don't think they are ready yet. They will break and the whole tide will have to reversion, making all tide middleware having to upgrade.
  • tide-default-header will have to go into tide. Can't be a separate crate. Same as above.
  • tide-compression is currently a separate crate. Then, will have to moved into tide. Can't be just a reexport. Meaning it makes dev tedious for graduating.
  • New middleware will have to be either in tide, or not. You can't graduate them easily rather then moving the entire code in due to how dependencies are structured.
  • Some one builds a middleware externally (say, tide-staticfile right now, hypothetically). We arrive at consensus that it's extremely useful to go into tide - Now you have to absorb the author's work into tide and move the whole code in.

tide-core + tide

  • tide-core lives minimally independently. World gets to use tide anyway and the whole bunch. User experience is exactly the same.
  • Middlewares uses tide-core - minimal set.
  • tide-cookies is independent. Depends on tide-core. Re-exported by tide. Makes changes, doesn't matter for any body else other that tide and people who depend on the end application. All middlewares authors don't care.
  • tide-default-header can be named, renamed, changed in whatever way. tide simply exports it if and when ready.
  • Makes life so much simpler for middleware authors as they don't ever have to worry about tide versions as they only care about tide-core which will be much less frequent and encourage middleware experimentation.
  • When some odd middleware implementations that need everything, they naturally can move to pulling the whole tide - though I think should be very rare with middlewares.
  • Some one builds a middleware externally (say, tide-staticfile right now, hypothetically). We arrive at consensus that it's extremely useful to go into tide - tide just depends and reexports, still crediting the original author - this may not happen, and we may just want to keep things in as well. But the flexibility immensely eases development.

From a user's perspective this is still just one dependency tide.

tide-core is only for middleware authors, tide developers eitherway.

@Nemo157
Copy link
Contributor

Nemo157 commented May 15, 2019

I suspect that a distinction between tide-core and tide doesn't buy us any extra semver mobility, because almost everyone would opt to use tide directly.

You can still directly refer to tide and get semver mobility, if you have crate foo depending on tide v0.1 and crate bar depending on tide v0.2 they will still have API compatibility as long as the public types they mention are those from tide-core v0.1 which both versions of tide depend on. If they mentioned specific middleware in their public APIs then there will be semver compatibility issues if those middleware have a breaking change, but it seems like most APIs should be built on the types from tide-core (even if they're referred to from a path starting at tide).

@Nemo157
Copy link
Contributor

Nemo157 commented May 15, 2019

# minimal
[dependencies]
tide = { version = "1.3.2", features = [ "bare" ] }

this should be

# minimal
[dependencies]
tide = { version = "1.3.2", default-features = false }

or you're going to have a lot of issues related to bad feature interactions. If you want all features on by default you just need to specify all features in the default feature (e.g. that's how I just setup async-compression so that users could choose which compression schemes to support while defaulting to having everything enabled).

@prasannavl
Copy link
Contributor

prasannavl commented May 15, 2019

@Nemo157 - Yes, this is the only way to allow for things to be in separate crates wihout tide-core as far as I know. I see this as being far more inconvenient rather than just using tide-core from a middleware perspective. This exactly what I mentioned while I said,

We can't have middleware and tide managed independently while being exposed from the tide crate, as that involves a cyclic dependency (unless we resort to more feature based dependency shenanigans here which could not be the most ergonomic)

Very simply put features flags are easy to miss, pain to manage, rather than just a simple tide-core and not worry, from a middleware author's perspective.

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

No branches or pull requests

10 participants