-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
[feat]: Added port already in use warning. #558
Conversation
👷 Deploy request for robyn pending review.Visit the deploys page to approve it
|
@sansyrox I think the feature became a bug for the tests in Windows 😅 . The GHA is running all the tests for windows on the same machine at the same time I guess that is why that port is already in use warning/error is there 🥲 . |
e6dfe14
to
2c5c843
Compare
@sansyrox Fixed all the requested changes. 🕺🏻 |
CodSpeed Performance ReportMerging #558 will not alter performanceComparing Summary
|
71f36b6
to
416bc97
Compare
robyn/processpool.py
Outdated
except OSError: | ||
return True | ||
except Exception: | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Mr-Sunglasses 👋
One suggestion - let us return errors here. It is a recommended way to
not do a generic catch all for the Exception
5588625
to
d04fb5e
Compare
Hey @Mr-Sunglasses 👋 Any updates on this PR ? |
6ae6e67
to
2ffd591
Compare
Added port already in use warning and added to ask the user for the prompt to use the new port.
Description
port-patch.mov
This PR fixes #508, #557