-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Refactor net gevent #18
Conversation
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.
I'm hesitant to make this large fix without a test in place. We can filter through issues and skip this until the test suite refactoring is started.
import struct | ||
|
||
import gevent | ||
import gevent.socket as socket |
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.
This is the reason I noted waiting on tests below. The documentation for this class says it will only block one greenlet and that is essential to being async. I would rather wait for tests. Otherwise socket wrapper should work like connection where it operates on an internal instance object which can be overridden in the subclass constructor. Long term this pattern is a good place for metaclass factories.
|
||
|
||
class GeventCursorEmpty(ReqlCursorEmpty, StopIteration): | ||
pass | ||
|
||
|
||
class Connection(net.Connection): |
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.
This would still need the old init where it provides the instance class.
I see you point. Then what if I mark this PR as blocked until the tests are in place? FYI, the integration test functionality is done with automatic droplet creation. I’ll open a PR tomorrow. |
Sounds good. I will try to carve out some time today to get some more work done on my open issues. |
Description
The
net_gevent
contains lot of style issues and duplicates somenet
classes. This PR adjusts the code base to be able to use the duplicated classes' ancestor fromnet
. Also fixing some styling issues.Please note that
Connection
class innet_gevent
moved up from the bottom, and theSocketWrapper
removed in favour ofnet
's own one. (The difference was the timeout/deadline parameter)Important
Please be careful with the review and check if I made any mistake. As far as I can see, no breaking stuff introduced with removing
SocketWrapper
fornet_gevent
.