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

Decoupled ZMQ from Mongrel2 in Mongrel2Connection object #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sethmurphy
Copy link
Collaborator

Mongrel2Connection implementation now extends new abstract ZMQConnection. All code is the same as before, just re-factored. It is now possible to extend ZMQConnection to create non Mongrel2 based connections using ZMQ also. This was done in preparation for a BrubeckConnection for peer to peer Brubeck Instance connections also using ZMQ.

… abstract ZMQConnection and extending it for Mongrel2Connection.
@sethmurphy
Copy link
Collaborator Author

This method of refactoring seemed to be the most backwards compatible way to decouple, but it does create an extra layer of hierarchy which could be avoided by using composition in the ZMQConnection instead by passing in a request object that could specific for the message format.

Does anyone think creating this extra layer of object hierarchy will be detrimental to Brubeck's performance? Performance should be the top priority in the connection layer.

@j2labs
Copy link
Owner

j2labs commented Nov 29, 2012

Hey Seth, I want to accept this. I haven't looked it since we last discussed. What is your current opinion?

@sethmurphy
Copy link
Collaborator Author

My current opinion, while useful, it creates too deep of class hierarchy.
It does need to be a class though, so since it instantiates only once that
may be OK.

As for the close method it was a pass and I left it as is. it would
probably be more proper to send an HTTP response. It may be best to leave
it unimplemented. Close has no real meaning and perhaps the programmer
should be responsible for sending a valid reason with send.

On Thu, Nov 29, 2012 at 10:19 AM, James Dennis [email protected]:

Hey Seth, I want to accept this. I haven't looked it since we last
discussed. What is your current opinion?


Reply to this email directly or view it on GitHubhttps://github.com//pull/81#issuecomment-10850920.

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.

2 participants