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

Wrap Application in custom monad for Raw endpoint #1349

Closed
wants to merge 2 commits into from

Conversation

maksbotan
Copy link
Contributor

Currently handler Raw endpoint for is just an Application: a value.
This value has no access to the context of an arbitrary monad that may
be used instead of the default Handler.

This commit changes handler type to m Application, allowing the user
to do arbitrary actions in m prior to generating an Application.

This is useful to do custom logic behind servants back while still
having access to the environment of user's monad.

@maksbotan
Copy link
Contributor Author

This is a breaking change, so maybe it makes sense to introduce a new combinator, like RawM, to have this behavior. However, I think the new behavior is "more right" and the users just have to migrate, the change is not that big (add return before your application).

@fisx
Copy link
Member

fisx commented Oct 12, 2020

I like it, but as you can tell we're a bit behind with processing PRs. Would you be interested in joining the maintainer team? :-)

If not, we'll get to this eventually. I agree with the breaking change, any other opinions?

Currently handler `Raw` endpoint for is just an `Application`: a value.
This value has no access to the context of an arbitrary monad that may
be used instead of the default `Handler`.

This commit changes handler type to `m Application`, allowing the user
to do arbitrary actions in `m` prior to generating an `Application`.

This is useful to do custom logic behind `servant`s back while still
having access to the environment of user's monad.
@maksbotan
Copy link
Contributor Author

Well, maintaining such an important library is a great responsibility...

Can I help with current problems? I'd love to see uverbs finally in master, for example.

@maksbotan
Copy link
Contributor Author

Actually I was able to generalize this even further, please take a look.

@amesgen
Copy link
Contributor

amesgen commented Oct 20, 2020

JFYI: There is https://github.com/cdepillabout/servant-rawm, which solves this exact problem.

I think it is a very good idea to integrate it/sth similiar in this repo!

In particular, it will make it easier to close cdepillabout/servant-rawm#7, as servant-auth can provide the required instance out of the box (without having to write a servant-auth-rawm compat package).

@maksbotan
Copy link
Contributor Author

Oops, perhaps I should've looked for prior art :)

However, newest version of this PR is actually better than that package, as it allows you to run m-based effects depending on the request. I've found it useful in my app.

@maksbotan
Copy link
Contributor Author

@fisx so how about co-maintaining? I could at least help with technical releases for version bounds and stuff.

cc @fizruk.

@maksbotan
Copy link
Contributor Author

I've just released 0.18.1 to Hackage. I believe this change will have to go into 0.19, as it's kinda breaking. What do you think?

@amesgen
Copy link
Contributor

amesgen commented Nov 4, 2020

I agree that the Raw rework should go in 0.19.

One nice feature of servant-rawm is that it allows to customize the HasDocs instance of RawM'. This should not be too hard to add to this PR.

@amesgen amesgen mentioned this pull request Mar 9, 2022
@tchoutri tchoutri closed this Jul 11, 2024
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

Successfully merging this pull request may close these issues.

4 participants