Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] bpo-39259: nntplib.NNTP/NNTP_SSL now reject timeout = 0 #17936
Conversation
This refactoring is interesting. I merged your other change to reject timeout=0, you should now rebase this PR. Currently, this PR is a refactoring and rejects timeout=0. I prefer to have one commit per change. |
def _create_socket(self, timeout): | ||
if timeout is not None and not timeout: | ||
raise ValueError('Non-blocking socket (timeout=0) is not supported') | ||
return socket.create_connection((self.host, self.port), timeout) |
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 10, 2020
Member
You should move sys.audit("nntplib.connect", self, host, port)
here :-)
usenetrc, timeout) | ||
|
||
def _create_socket(self, timeout): | ||
self.sock = NNTP._create_socket(self, timeout) |
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 10, 2020
Member
self.sock = NNTP._create_socket(self, timeout) | |
self.sock = super()._create_socket(timeout) |
finally: | ||
self.sock.close() | ||
self.ssl_context = ssl_context | ||
NNTP.__init__(self, host, port, user, password, readermode, |
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 10, 2020
Member
NNTP.__init__(self, host, port, user, password, readermode, | |
super().__init__(host, port, user, password, readermode, |
|
||
def _create_socket(self, timeout): | ||
self.sock = NNTP._create_socket(self, timeout) | ||
self.sock = _encrypt_on(self.sock, self.ssl_context, self.host) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Jan 10, 2020
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
corona10 commentedJan 10, 2020
•
edited by bedevere-bot
https://bugs.python.org/issue39259