nitrogen / simple_bridge

A simple, standardized interface library to Erlang HTTP Servers.
MIT License
112 stars 76 forks source link

cowboy_dispatch_function app configuration variable not honored #43

Closed erlanger closed 9 years ago

erlanger commented 9 years ago

Simple bridge does not use the cowboy_dispatch_function mentioned in the sample configuration, but instead honors cowboy_dispatch_fun; either the code or the config file needs to be ammended.

I have no preference, but I think short names are better :)

See line 57 of src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl here: https://github.com/nitrogen/simple_bridge/blob/cede863c96356a9f12c90976e3845d3f2c17710d/src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl#L57e

choptastic commented 9 years ago

Oh wow, you're totally right. Thanks for pointing that out. I'll get this fixed shortly.

On Thu, Jan 15, 2015 at 9:01 PM, erlanger notifications@github.com wrote:

Simple bridge does not use the cowboy_dispatch_function mentioned in the sample configuration, but instead honors cowboy_dispatch_fun; either the code or the config file needs to be ammended.

I have no preference, but I think short names are better :)

See line 57 of src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl here:

https://github.com/nitrogen/simple_bridge/blob/cede863c96356a9f12c90976e3845d3f2c17710d/src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl#L57e

— Reply to this email directly or view it on GitHub https://github.com/nitrogen/simple_bridge/issues/43.

Jesse Gumm Owner, Sigma Star Systems 414.940.4866 || sigma-star.com || @jessegumm

choptastic commented 9 years ago

Looks like I made the mistake in the config sample, seeing as webmachine does it the same way. https://github.com/nitrogen/simple_bridge/blob/cede863c96356a9f12c90976e3845d3f2c17710d/src/webmachine_bridge_modules/webmachine_simple_bridge_sup.erl#L51

I also prefer the shorter version: cowboy_dispatch_fun and webmachine_dispatch_fun

Thanks again,

-Jesse

On Thu, Jan 15, 2015 at 9:03 PM, Jesse Gumm gumm@sigma-star.com wrote:

Oh wow, you're totally right. Thanks for pointing that out. I'll get this fixed shortly.

On Thu, Jan 15, 2015 at 9:01 PM, erlanger notifications@github.com wrote:

Simple bridge does not use the cowboy_dispatch_function mentioned in the sample configuration, but instead honors cowboy_dispatch_fun; either the code or the config file needs to be ammended.

I have no preference, but I think short names are better :)

See line 57 of src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl here:

https://github.com/nitrogen/simple_bridge/blob/cede863c96356a9f12c90976e3845d3f2c17710d/src/cowboy_bridge_modules/cowboy_simple_bridge_sup.erl#L57e

— Reply to this email directly or view it on GitHub https://github.com/nitrogen/simple_bridge/issues/43.

Jesse Gumm Owner, Sigma Star Systems 414.940.4866 || sigma-star.com || @jessegumm

Jesse Gumm Owner, Sigma Star Systems 414.940.4866 || sigma-star.com || @jessegumm

erlanger commented 9 years ago

You're welcome, thanks for the good work!

erlanger commented 9 years ago

BTW, I think it would be good to point out in the sample configuration that the cowboy_dispatch_fun needs to return a compiled route (with cowboy_router:compile()) and that the user should probably add a route like this at the end: {'_', cowboy_simple_bridge_anchor,[]}

without this route nitrogen can't do its dynamic dispatching of modules.

I was debating in my head if simple_bridge should automatically add this route at the end or not, but it is probably best left to be done by the user.

choptastic commented 9 years ago

Very good point. Thank you!

I'm sure at the time I was writing the docs, I thought "yeah, that's totally obvious", but when you mention it, it's completely not at all obvious. So yeah, good call on that!

On Thu, Jan 15, 2015 at 9:33 PM, erlanger notifications@github.com wrote:

BTW, I think it would be good to point out in the sample configuration that the cowboy_dispatch_fun needs to return a compiled route (with cowboyrouter:compile()) and that the user should probably add a route like this at the end: {'', cowboy_simple_bridge_anchor,[]}

without this route nitrogen can't do its dynamic dispatching of modules.

I was debating in my head if simple_bridge should automatically add this route at the end or not, but it is probably best left to be done by the user.

— Reply to this email directly or view it on GitHub https://github.com/nitrogen/simple_bridge/issues/43#issuecomment-70203733 .

Jesse Gumm Owner, Sigma Star Systems 414.940.4866 || sigma-star.com || @jessegumm