Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

bpo-37322: Fix ResourceWarning and exception handling in test (GH-25553)#25553

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tiran merged 2 commits intopython:masterfromtiran:bpo-37322-resourcewarning
Apr 24, 2021

Conversation

tiran
Copy link
Member

@tirantiran commentedApr 23, 2021•
edited by bedevere-bot
Loading

Handle all OSErrors in a single block. OSError also takes care of
SSLError and socket's connection errors.

Partly reverts commitfb7e750. The
threaded connection handler must not raise an unhandled exception.

https://bugs.python.org/issue37322

@tirantiran added testsTests in the Lib/test dir skip news needs backport to 3.8 needs backport to 3.9only security fixes labelsApr 23, 2021
@tirantiran added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@tiran for commit a3594aab883ba2e1d47ab853c1301d68c66fe2e3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@tiran
Copy link
MemberAuthor

Buildbots failed to trigger, let's try again.

@tirantiran added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@tiran for commit a3594aab883ba2e1d47ab853c1301d68c66fe2e3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@pablogsalpablogsal added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@pablogsal for commit a3594aab883ba2e1d47ab853c1301d68c66fe2e3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
Comment on lines -2484 to -2501
# On Windows sometimes test_pha_required_nocert receives the
# PEER_DID_NOT_RETURN_A_CERTIFICATE exception
# before the 'tlsv13 alert certificate required' exception.
# If the server is stopped when PEER_DID_NOT_RETURN_A_CERTIFICATE
# is received test_pha_required_nocert fails with ConnectionResetError
# because the underlying socket is closed
Copy link
Contributor

@matthewhughes934matthewhughes934Apr 23, 2021•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is there a risk of this failing on Windows, or would this be covered by the CI suite? I'm not too familiar with the code (nor windows) so I kept away from this when I was looking around this issue.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Python has pre-commit CI and post-commit buildbots for all major platforms including multiple flavors of Windows.

It doesn't make any sense the raise an exception here. The exception just breaks the handler thread and does not even end up in the test case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks! That clears things up

@tirantiranforce-pushed thebpo-37322-resourcewarning branch froma3594aa to04260c4CompareApril 23, 2021 18:06
@tirantiran added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@tiran for commit 04260c43c5879a7ef3800c045d9dfacab5c225a1 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@tirantiran added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@tiran for commit 62653f2715fc4e8f9c991b867cee5825fc4d0077 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 23, 2021
@tirantiranforce-pushed thebpo-37322-resourcewarning branch from62653f2 tob0b4024CompareApril 24, 2021 04:57
Handle all OSErrors in a single block. OSError also takes care ofSSLError and socket's connection errors.Partly reverts commitfb7e750. Thethreaded connection handler must not raise an unhandled exception.
Revert73ea546, increase logging, and improve stability of test.
@tirantiranforce-pushed thebpo-37322-resourcewarning branch fromb0b4024 to25fd780CompareApril 24, 2021 04:59
@tirantiran added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 24, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@tiran for commit25fd780 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 24, 2021
@tirantiran changed the titlebpo-37322: Fix ResourceWarning and exception handling in testbpo-37322: Fix ResourceWarning and exception handling in test (GH-25553)Apr 24, 2021
@tirantiran merged commitc8666cf intopython:masterApr 24, 2021
@miss-islington
Copy link
Contributor

Thanks@tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

@tirantiran deleted the bpo-37322-resourcewarning branchApril 24, 2021 07:17
@miss-islington
Copy link
Contributor

Sorry,@tiran, I could not cleanly backport this to3.9 due to a conflict.
Please backport usingcherry_picker on command line.
cherry_picker c8666cfa7cdc48915a14cd16095a69029720736a 3.9

@miss-islington
Copy link
Contributor

Sorry@tiran, I had trouble checking out the3.8 backport branch.
Please backport usingcherry_picker on command line.
cherry_picker c8666cfa7cdc48915a14cd16095a69029720736a 3.8

@bedevere-bot
Copy link

GH-25572 is a backport of this pull request to the3.9 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.9only security fixes labelApr 24, 2021
@bedevere-bot
Copy link

GH-25573 is a backport of this pull request to the3.8 branch.

tiran added a commit to tiran/cpython that referenced this pull requestMay 2, 2021
…ythonGH-25553)Revert 73ea546, increase logging, and improve stability of test.Handle all OSErrors in a single block. OSError also takes care ofSSLError and socket's connection errors.Partly reverts commit fb7e750. Thethreaded connection handler must not raise an unhandled exception..(cherry picked from commitc8666cf)Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit to tiran/cpython that referenced this pull requestMay 2, 2021
…ythonGH-25553)Revert 73ea546, increase logging, and improve stability of test.Handle all OSErrors in a single block. OSError also takes care ofSSLError and socket's connection errors.Partly reverts commit fb7e750. Thethreaded connection handler must not raise an unhandled exception..(cherry picked from commitc8666cf)Co-authored-by: Christian Heimes <christian@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@matthewhughes934matthewhughes934matthewhughes934 left review comments

@ethanfurmanethanfurmanAwaiting requested review from ethanfurman

@isidenticalisidenticalAwaiting requested review from isidentical

Assignees

@tirantiran

Labels
skip newstestsTests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@tiran@bedevere-bot@miss-islington@matthewhughes934@pablogsal@the-knights-who-say-ni
[8]ページ先頭
©2009-2025 Movatter.jp