Skip to content

/cpython

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-37228: Fix warnings in test_asyncio.test_base_events #17577

Merged
merged 1 commit into from Dec 12, 2019
Merged

bpo-37228: Fix warnings in test_asyncio.test_base_events #17577

merged 1 commit into from Dec 12, 2019

Conversation

@aeros
Copy link
Member

aeros commented Dec 12, 2019 •
edited

Co-authored-by: @tirkarthi

The security patch #17311 (which was backported to 3.8, 3.7, and 3.6) added a few subtle warnings, including a ResourceWarning and two DeprecationWarnings in test_asyncio.test_base_events. @tirkarthi discovered the warnings, and proposed a fix for the ResourceWarning. See the bpo issue for more details.

https://bugs.python.org/issue37228
@aeros
Fix warnings in test_asyncio.test_base_events
Loading status checks…
8d59e8a 
Co-authored-by: tirkarthi
@aeros aeros requested review from 1st1 and asvetlov as code owners Dec 12, 2019
@aeros aeros requested review from ned-deily and ambv Dec 12, 2019
@ambv

This comment has been minimized.

Sign in to view
Copy link
Contributor

ambv commented Dec 12, 2019

Wasn't the point of the security fix to not use reuse_addr=?
@aeros

This comment has been minimized.

Sign in to view
Copy link
Member Author

aeros commented Dec 12, 2019 •
edited

@ambv

Wasn't the point of the security fix to not use reuse_addr=?

The primary point of the security fix was to specifically remove usage of reuse_addr=True for create_datagram_endpoint() and no longer use it as a default. But as a part of it we also deprecated the reuse_address parameter itself since it was turned into a no-op. This PR is simply addressing some minor test changes that I missed, which @tirkarthi picked up on by running the tests with -Wall.

I accidentally introduced a couple deprecation warnings that were raised from previous tests that still had reuse_address=False, which should have been removed. Also, in the actual test for the deprecation warning, the transport and protocol were not closed, leading to a resource warning.

It wouldn't be the end of the world if this didn't make it into the next releases, but it would definitely be preferable to not have additional warnings lingering in the regression tests. Especially in this case where the fix is quite simple.
@ambv ambv merged commit 1988344 into python:master Dec 12, 2019
4 checks passed
4 checks passed
Azure Pipelines PR #20191212.2 succeeded
Details
bedevere/issue-number Issue number 37228 found
Details
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 12, 2019

@ambv: Please replace # with GH- in the commit message next time. Thanks!
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Dec 12, 2019

Thanks @aeros for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖
@ambv

This comment has been minimized.

Sign in to view
Copy link
Contributor

ambv commented Dec 12, 2019

Thanks! ✨ 🍰 ✨
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 12, 2019

GH-17579 is a backport of this pull request to the 3.8 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
9499a7b 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 12, 2019

GH-17580 is a backport of this pull request to the 3.7 branch.
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 12, 2019

GH-17581 is a backport of this pull request to the 3.6 branch.
miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
e0afcd7 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
0230f47 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
@tirkarthi

This comment has been minimized.

Sign in to view
Copy link
Contributor

tirkarthi commented Dec 12, 2019

Thanks @aeros :)
ambv added a commit that referenced this pull request Dec 12, 2019
@miss-islington @aeros @ambv
Fix warnings in test_asyncio.test_base_events (GH-17577) (#17581)
Loading status checks…
b35ec00 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
sthagen added a commit to sthagen/cpython that referenced this pull request Dec 12, 2019
@sthagen
Merge pull request #94 from python/master
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
485bfd7 
Fix warnings in test_asyncio.test_base_events (python#17577)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ambv ambv

@1st1 1st1

@asvetlov asvetlov

@ned-deily ned-deily

Assignees
No one assigned
Labels
CLA signed expert-asyncio skip news type-bugfix type-tests
Projects
None yet
Milestone
No milestone
6 participants
@aeros @ambv @bedevere-bot @miss-islington @tirkarthi @the-knights-who-say-ni
You can’t perform that action at this time.