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

Added handler_map to SocketIO class #994

Closed
wants to merge 1 commit into from

Conversation

bpengu1n
Copy link

@bpengu1n bpengu1n commented Jun 2, 2019

Added handler_map to SocketIO class permitting iteration over endpoints and their handler functions i.e. for autodoc purposes.

endpoints and their handler functions i.e. for autodoc purposes.
@bpengu1n
Copy link
Author

bpengu1n commented Jun 2, 2019

Resolved concerns raised in #573

@bpengu1n
Copy link
Author

bpengu1n commented Jun 2, 2019

@miguelgrinberg Kept this very simple, just a list to iterate message -> handler. Works great with the Sphinx plugin I'm developing to generate API docs for Flask_SocketIO endpoints. Happy to address any modifications you think should occur!

@miguelgrinberg
Copy link
Owner

@bpengu1n Thanks, this helps me better understand what your needs are. Is this Sphinx plugin that you are writing something internal for your project or is it a public available plugin? Can I take a quick look at it if the latter?

The change that you made is safe, but can't help but bother a bit about duplicating the list of handlers, so I'd like to explore if there is a better way.

@bpengu1n
Copy link
Author

bpengu1n commented Jun 3, 2019

@miguelgrinberg That's understandable! I intend to make the plugin public, basing it currently on the 'autohttp' plugin, though I will likely end up refactoring it into an entirely new plugin as autohttp/httpdomain likely won't end up being a good fit for it.
See https://github.com/bpengu1n/httpdomain/blob/flask_socketio/sphinxcontrib/autohttp/flask_socketio.py

@miguelgrinberg
Copy link
Owner

@bpengu1n after some thought I decided to add the @wraps decorator as initially suggested in #573.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants