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

Customizable protocol #209

Merged
merged 9 commits into from
Jan 3, 2017
Merged

Customizable protocol #209

merged 9 commits into from
Jan 3, 2017

Conversation

38elements
Copy link
Contributor

@38elements 38elements commented Dec 22, 2016

In the current implementation, the developer can not change the behavior of the protocol.
By implementing a subclass of HttpProtocol and making it an argument of the run function,
the developer can change the behavior when data is received and the behavior when an error occurs.
This is related #115 and #208 .

def serve(host, port, request_handler, error_handler, before_start=None,
after_start=None, before_stop=None, after_stop=None,
debug=False, request_timeout=60, sock=None,
def serve(protocol, host, port, request_handler, error_handler,
Copy link
Member

Choose a reason for hiding this comment

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

Would it be smart to put this at the front of the arguments list and possibly break functionality for anyone who uses this? I think putting this at the end and defaulting it to our own HttpProtocol would be better in this case.

@38elements
Copy link
Contributor Author

@seemethere
Thank you for reviewing.
Your pointing out is correct.
I fixed this code.

@seemethere seemethere added this to the 0.2.0 milestone Dec 24, 2016
Copy link
Member

@seemethere seemethere left a comment

Choose a reason for hiding this comment

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

Is there a way to write a test for this? Also could you include an example of a custom HttpProtocol implementation?

@38elements
Copy link
Contributor Author

38elements commented Dec 25, 2016

@seemethere
Of course I will make document, example and test.

@38elements
Copy link
Contributor Author

@seemethere
I made document, example and test.

@seemethere seemethere merged commit 4ccc782 into sanic-org:master Jan 3, 2017
@seemethere seemethere mentioned this pull request Jan 3, 2017
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.

None yet

2 participants