-
-
Notifications
You must be signed in to change notification settings - Fork 597
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
Namespace catchall #1288
Namespace catchall #1288
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1288 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 31 31
Lines 2405 2423 +18
Branches 420 421 +1
=========================================
+ Hits 2405 2423 +18 ☔ View full report in Codecov by Sentry. |
Added more tests, which should fix the codecov. I don't know why the lint is failing -- I don't have any experience with tox and no error message is given. @miguelgrinberg can you take a look? Also, with a catchall namespace handler, you still need to explicitly set |
@mooomooo the flake8 errors are these:
Regarding the connection question, the tests are connecting directly at the client manager level. The logic to reject a connection based on namespace are in the server, before the client manager is invoked. This is why the tests allow you to connect on random namespaces. See these lines. |
Got it, makes sense. I think this should be good for review now! |
Looks good, nothing more to comment on. I need to test this out a little bit, write the documentation and then I'll merge it, hopefully within the next week or so. Thanks! |
Merged! I copied your server implementation to the client and also added some documentation. Thanks! |
Thank you for working on this enhancement. Has this code been released yet? python-socketio/src/socketio/async_server.py Line 617 in 8012413
I'm trying to get catch-all namespaces for event handler to work.
Also, for connect and disconnect events, I could get catch-all namespaces to work by looping through list of namespaces and defining decorators. |
@kdNew23 Yes, this is in the latest release. |
Closes #1285
Note: Existing tests all pass,
but no additional tests have been added validate wildcard functionality.Namespace catchall tests added and also passed