Skip to content
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

bpo-39259: poplib.POP3/POP3_SSL now reject timeout = 0 #17912

Merged
merged 4 commits into from Jan 10, 2020

Conversation

@corona10
Copy link
Member

corona10 commented Jan 9, 2020

@corona10 corona10 requested a review from python/email-team as a code owner Jan 9, 2020
@corona10 corona10 requested a review from vstinner Jan 9, 2020
@corona10 corona10 force-pushed the corona10:bpo-39259 branch from aa65a71 to 9d99211 Jan 9, 2020
@@ -505,17 +505,17 @@ def server(self, evt, serv):

def testTimeoutDefault(self):
self.assertIsNone(socket.getdefaulttimeout())
socket.setdefaulttimeout(30)
socket.setdefaulttimeout(test_support.SHORT_TIMEOUT)

This comment has been minimized.

Copy link
@vstinner

vstinner Jan 9, 2020

Member

When I added timeout constants to the test.support module, I decided to leave this test unchanged. I understood that the test doesn't connect to real socket. Or does it? I'm not sure. If it does, LOOPBACK_TIMEOUT is preferred.

This comment has been minimized.

Copy link
@corona10

corona10 Jan 9, 2020

Author Member

self.welcome = self._getresp()

As you can see poplib client's constructor actually makes a connection to the server and get a response.

self.thread = threading.Thread(target=self.server, args=(self.evt,self.sock))

So this test intends to check whether poplib client is set by a global socket timeout and also check whether the connection is succeeded.

@corona10

This comment has been minimized.

Copy link
Member Author

corona10 commented Jan 9, 2020

@vstinner I have made the requested changes; please review again

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Jan 9, 2020

Thanks for making the requested changes!

: please review the changes made to this pull request.

Doc/library/poplib.rst Outdated Show resolved Hide resolved
Co-Authored-By: Victor Stinner <vstinner@python.org>
Doc/library/poplib.rst Outdated Show resolved Hide resolved
Co-Authored-By: Victor Stinner <vstinner@python.org>
Copy link
Member Author

corona10 left a comment

@vstinner I 've updated the PR.
For other modules, I will submit the PR seperately.

@vstinner vstinner merged commit c39b52f into python:master Jan 10, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200110.6 succeeded
Details
bedevere/issue-number Issue number 39259 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.