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

Fold some of this into Pedestal? #2

Open
ohpauleez opened this issue Jul 27, 2016 · 3 comments
Open

Fold some of this into Pedestal? #2

ohpauleez opened this issue Jul 27, 2016 · 3 comments

Comments

@ohpauleez
Copy link

Hi @frankiesardo!

I was just strolling through some routing ideas and I saw a note I left myself to come back here and take a look. Now that Pedestal's routing is a separate module, you might find some pieces you could reuse from there. But more importantly...

Soon Pedestal's core route pieces won't be tied to HTTP verbs, and I'd like your input on other ideas and use-cases you think I should aware of in this area.

Feel free to close this issue and push the conversation to another medium!

@frankiesardo
Copy link
Owner

Hey @ohpauleez! Thanks for checking in, I'm happy to contribute to the discussion however I can.

My gut feeling about verb maps is that they should be kept out of the route tables. Strictly speaking, they are not part of the routing and they tie pedestal.routes with something too domain specific.

tripod would be easily foldable back into pedestal if it supported a one-to-one mapping between routes and interceptors:

(def home ...)
(def pets ...)

(def routes
  [["/" home
    ["/pets" pets]]])

Which makes it trivial to use in other context (e.g cljs single page apps, ring+liberator, message/topic dispatch etc.)

Pedestal could still provide a convenient way to specify verb maps for specific domains like http:

(def pets (interceptor.http/handler {:get get-all-pets
                                     :post create-new-pet}))

(def home (interceptor.http/handler {:get get-home-page}))

(def routes
  [["/" home
    ["/pets" pets]]])

interceptor.http/handler can return an interceptor that does opportune dispatching and responds with a 405 if need to be. Do you think that could be an option for pedestal moving forwards?

@ohpauleez
Copy link
Author

Great feedback all around. More than likely, the route compiler will get more intelligent about verbs. It is my plan to support a set of verbs passed in as an argument during the route tree compilation. If no verbs are passed in, you'll get a one-to-one mapping. Otherwise verbs will be compiled into the tree.

As it stands right now, the prefix-tree (and the map-tree) only route on path, and then perform a "constraint routing" which includes verb routing. That is, one-to-one routing is the norm (already), and the idea is to expose more control over the secondary constraint routing.

Feel free to close this issue if you're satisfied :)
Otherwise, keep slinging feedback at me!

@frankiesardo
Copy link
Owner

If the one-to-one use case is supported then I've only one small concern about it.

I loved how you evolved Pedestal in the last few iterations, progressively hiding away excessive customisation in favour of a recommended way of doing things (one preferred way of creating interceptors, one preferred route syntax, etc.)

The change you propose now would create different valid outputs after route expansion, which is annoying for external tools that consume it like route-swagger and provides again many different ways of doing things that confuse end users. (well, it is confusing for me and I consider myself a proficient pedestal user)

One positive feedback I've received for tripod is that is very lightweight (in cognitive terms I guess) and doesn't have all that parametrisation that most of the users don't really need. I think pedestal could move forwards by removing stuff rather than adding entry points.

Nothing an external library cannot cope with, btw, so it's not dramatic. But my point being: pedestal will keep on feeling huge just because it has so many dsls nobody has really time to learn about.

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

No branches or pull requests

2 participants