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

Multiple listeners #44

Closed

Conversation

majastanislawska
Copy link
Contributor

Hi.
There is a complete (IMO) multiple_listeners patch.
It introduces a bit of incompatibility as it introduces new module parameter for controllers, but IMO this approach is easier to understand for users, easier to use and much cleaner in implementation.
'Instance' variable holds atom 'default' when multiple listeners configuration is not used.

@evanmiller
Copy link
Contributor

You still have not answered my question. Why can't we just use the Req object to distinguish SSL and non-SSL traffic? Using the server name defined in the configuration seems like bad design because it reduces application re-usability.

As for the mochiweb change: Did you send this patch upstream? I am generally hesitant to change non-Boss code unless I know exactly what is going on. I would want feedback from the mochiweb team because there might be a good reason for using a timeout in the accept loop.

Finally if we go with the "servers" block we may want to eliminate the port/host/ssl options from the root config, but this can wait. PS- I like the "engine" name!

@@ -393,13 +407,17 @@ process_result(_, {ok, Payload, Headers}) ->
{200, [{"Content-Type", proplists:get_value("Content-Type", Headers, "text/html")}
|proplists:delete("Content-Type", Headers)], Payload}.

<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trees not merged?

@majastanislawska
Copy link
Contributor Author

@kotedo Regular port and server options are for 'backward compatibility', They are ignored when 'servers' list is defined.
I think it has been described in doc, if not clear enough i'll try to figure out something better.

@evanmiller:
I dont think that identifier of listener entity is really part of request, and the role of that identifier can be more than just distinguishing between ssl and not-ssl. This can be forntend, backend or customer1,customer2 etc.
This way of passing instance name allows adding in future listener specific routes.
About application reusability: You may look at it from other direction, listener have to be named a specific way to acheive some funcionality.
I can imagine that apps like cb_admin can have parameter like 'allowed_listeners' which will be checked in _before if is executed on right listener (admin or backend interface) and return some redirect or notfound if not.
Same thing for some shopping cart or payment app and requirement for ssl.
about mochiweb: i just openned issue on mochi's github.
Sorry for delay i had to focus on something else for a while.

@evanmiller
Copy link
Contributor

Hmm. Perhaps ALL configuration should be server-specific? So automatically we get "allowed listeners" because applications are listener-specific. Then we could provide an API like boss_env:get_server_env('server_name') if you really want to get the server name (or other user-defined config variables). Then you could also have two instances of the same application running with independent data stores as long as they were hosted on different listeners. But then perhaps servers could be associated with domain names and not IP addresses; the Nginx configuration model could be a good example to follow. I need to think about this some more.

@majastanislawska
Copy link
Contributor Author

i dont like idea of CB becoming platform for virtual servers hosting, it is a bit too far IMO.
In my usecase i need access to same datastore in all listeners, especially session data, and i need that some urls are reachable on specific listener only and not reachable on others.

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