-
-
Notifications
You must be signed in to change notification settings - Fork 412
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] MultiVerb #1454
Comments
This really looks awesome, thank you very much for taking the time to bring it here. :) |
Do you support servant-auth package? |
not sure, but i doubt it. it's been written for https://github.com/wireapp/wire-server, and we use our own auth code. (that's also the reason do you want to give a shot at a PR? :) |
Coming back to this ticket... I consider it yet another incentive to try and unify |
@alpmestan @jkarni During the first two weeks of July, Scrive & Wire will work on this. We'd appreciate any half-baked idea, general direction, grand design or midsummer night's dream that you may have had about this. |
also i might use my access to this and possibly other repos to merge PRs and release new servant versions, if we all agree we're happy with the changes. if you want to be involved let us know! |
I don't think we have any blockers. What I am most worried about is whether all the version bounds are set correctly. For example, servant-client-core should depend on the new major release of servant, see #1758. I don't think our CI tests that this is set correctly. So we must be careful. The issue is that when a PR is merged, our machinery has no idea whether it is breaking/not-breaking compared to previous versions. I don't know of a way to have CI check that. You'd have to compare APIs across versions. So #1604 changed the public API but we didn't have a way to remind ourselves of that. But at least version bounds can be adjusted after release... So if we get it wrong, it can be fixed. Let's just release all packages though, instead of trying to be smart. |
OK so let me attempt to summarize what we had in mind back in the days, when chewing on this with Oleg. First off: what problem is this all trying to solve? Well that's easy. The basic machinery in servant gives a lot of information about one execution path of the handler, which usually represents some kind of "happy path". Client functions or documentation do not get a chance to know or say anything about the potential bad outcomes, or "alternative happy paths" or anything else than that one main happy path. OTOH, if we make everything "uverb-like" by default, then I suspect the hello world tutorial will scare the hell out of everybody. So what we were after was something that would "look simple in the simple case", but would be built on top of a flexible core that would allow users to talk about the exhaustive list of outcomes for an endpoint. #841 and it particular Oleg's work/branch might be good inspirations for the interested souls, in addition to UVerb/MultiVerb. He was trying to get a new I'd say the uverb/multiverb packages figured out most of the tricky stuffs, but it's really the consolidation/unification bit that is missing. And then one can wonder how far this should go. Can we make arbitrary elements of the usually-static info dynamic? "this endpoint returns X in JSON or Y in CSV, but the status code might be anything" or whatever. Feels like there's a revised "grammar" for servant hiding in there enabling all this. But no need to go that far. It's just my usual way of determining whether something I came up with is a pile of hacks or a solid bundle of abstractions: can it go beyond the standard use cases, "naturally" ? |
After some time working on #1766, I think the for sake of transition it is better if we first get MultiVerb into Servant without using it to replace the foundations of Verb. Especially the way it handles Headers is incompatible (although more ergonomic), so it will take some migration effort from the consumers' part. I'll feel more comfortable with |
In https://github.com/wireapp/wire-server, we have recently introduced a new approach for representing endpoints that can return multiple responses (including error responses), called
MultiVerb
. It has more or less the same function asUVerb
, but avoids HTTP-specific concepts and types leaking into the handler. It also solved our issues with empty content that I described in #1433.You can find the whole implementation here, and usage examples here and here.
If people think this is a good idea, we might be able to invest some time in making this mergeable.
The text was updated successfully, but these errors were encountered: