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
Handle family
passed as keyword argument
#75
Conversation
@miketheman can this get merged, or are there any concerns? Instead of this: def __new__(cls, *args, **kwargs): We could perhaps do: def __new__(cls, family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None) |
Hi @emontnemery ! I'm lacking context as to why this is an improvement on the existing behavior, as well as any tests to validate the change works as intended. |
The current code throws if the socket is created with its parameters passed as keywords instead of as positional arguments, this PR fixes that by handling both cases. I've added testcases where |
pytest_socket.py
Outdated
@@ -86,7 +86,10 @@ class GuardedSocket(socket.socket): | |||
""" socket guard to disable socket creation (from pytest-socket) """ | |||
def __new__(cls, *args, **kwargs): |
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.
Maybe this should be changed to def __new__(cls, family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None)
instead, then we can simply check family
, instead of handling both positional and keyword arguments cases.
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.
Interesting - maybe this should be more compatible with the defaults of the underlying class? https://github.com/python/cpython/blob/59435eea08d30796174552c0ca03c59b41adf8a5/Lib/socket.py#L220
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.
OK, updated to mimic the CPython implementation, i.e.:
def __new__(cls, family=-1, type=-1, proto=-1, fileno=None)
Code Climate has analyzed commit fc0ba71 and detected 0 issues on this pull request. View more on Code Climate. |
Thanks for working through this! |
Handle
family
passed as keyword argument tosocket
constructor