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

Drop python3.5 #1396

Closed
wants to merge 27 commits into from
Closed

Drop python3.5 #1396

wants to merge 27 commits into from

Conversation

wgwz
Copy link
Contributor

@wgwz wgwz commented Feb 10, 2020

Continues @pquentin's work from #1390
Closes #75

@wgwz
Copy link
Contributor Author

wgwz commented Feb 10, 2020

I was able to reproduce the issues locally with both 3.7-dev and 3.8-dev. It looks like the issues with the tests are all related to this error:

RuntimeError: cannot reuse already awaited aclose()/athrow()

I'm not sure how to proceed from here. I found this:

Any pointers? Maybe here is a good place to start?

@pquentin
Copy link
Member

@wgwz Thanks for working on this! I think we should just revert that commit for now, and open an issue to worry about it later.

@njsmith Assuming we'll soon have a working pull request here, we should first release Trio with just a warning as mentioned in #1390, right?

@njsmith
Copy link
Member

njsmith commented Feb 11, 2020

we should first release Trio with just a warning as mentioned in #1390, right?

I think that would be ideal. It could be like, merge the warning, release, and then immediately merge this PR within a 10 minute span, though.

@njsmith
Copy link
Member

njsmith commented Feb 11, 2020

The pypy failure looks real.

@njsmith
Copy link
Member

njsmith commented Feb 11, 2020

The correct bpo link for the new aclose/athrow thing is: https://bugs.python.org/issue39386

@njsmith
Copy link
Member

njsmith commented Feb 11, 2020

OK and I tracked down the cannot reuse already awaited aclose()/athrow() problem... it's a genuine regression in the CPython dev branches, that was introduced in a PR that I approved (whoops). Upstream bug filed here: https://bugs.python.org/issue39606

@njsmith
Copy link
Member

njsmith commented Feb 12, 2020

Phew, it's lucky we dug into that cannot reuse already awaited aclose()/athrow() thing... the problematic change was scheduled to be released in 3.8.2 on Monday. So it's pure luck that this PR came along this week and caught the regression before it could make it into a released Python... anyway I submitted a fix upstream at python/cpython#18475, in case anyone is curious.

@njsmith
Copy link
Member

njsmith commented Feb 14, 2020

That fix is merged upstream now, so sometime in the next day or so the Travis builds should update to include it, and the tests here should start passing.

Is there any place that would be good to see how to issue the type of deprecation warning you are looking for here? I've never done that before.

I don't have an example, no, but I guess it should just require adding something like this to the bottom of trio/__init__.py?

import sys
if sys.version_info < (3, 6):
    _deprecate.warn_deprecated("Support for Python 3.5", "0.14", issue=75, instead="Python 3.6+")
del sys

It might also need some fiddling with our pytest warnings filters to keep the tests passing.

@pquentin pquentin closed this Feb 15, 2020
@pquentin pquentin reopened this Feb 15, 2020
@wgwz
Copy link
Contributor Author

wgwz commented Feb 15, 2020

It looks like the pypy nightly branch is failing because of these 3.6 additions not being present: https://docs.python.org/3/whatsnew/3.6.html#socket

Related: 4f3030f

@pquentin
Copy link
Member

@wgwz Right, 4f3030f should go away if it does not exist on PyPI. Can you please revert it? Sorry about that.

Also, thanks for the deprecated warning commit! However it should go in another pull request, so that we can merge it and release it before merging this one.

@njsmith
Copy link
Member

njsmith commented Feb 19, 2020

I filed a bug with PyPy about the missing socket constants: https://foss.heptapod.net/pypy/pypy/issues/3171

@njsmith
Copy link
Member

njsmith commented Feb 19, 2020

We should add a comment next to the SO_PROTOCOL/SO_DOMAIN fallback definitions that links to that PyPy issue, so that later on we can figure out why the code is there :-)

@wgwz
Copy link
Contributor Author

wgwz commented Feb 19, 2020

I'm a bit puzzled by the latest CI failure..

@njsmith
Copy link
Member

njsmith commented Feb 19, 2020

I'm a bit puzzled by the latest CI failure..

It's a mysterious issue that happens occasionally for reasons unknown: #200 (comment)

It doesn't seem to be related to this PR though, so we can just re-run the CI here.

@njsmith njsmith closed this Feb 19, 2020
@njsmith njsmith reopened this Feb 19, 2020
@njsmith
Copy link
Member

njsmith commented Apr 27, 2020

Now that v0.14.0 is out, including the 3.5 deprecation, I guess this is unblocked. (Sorry for the long delay there!)

I think we can be relatively aggressive about dropping support here. For most deprecations, if folks haven't updated, then removing the code breaks them. But thanks to the magic of Python-Requires: metadata, in this case when we remove the code then folks who haven't updated just remain on the old version.

@njsmith njsmith closed this Apr 27, 2020
@njsmith njsmith reopened this Apr 27, 2020
@wgwz
Copy link
Contributor Author

wgwz commented Apr 27, 2020

Hey @njsmith, is there anything I can do to help here? I've somewhat lost track of where we left off :-)

@pquentin
Copy link
Member

The idea is to fix the conflict and remove code that's no longer covered. I can take over from here if you want.

@wgwz
Copy link
Contributor Author

wgwz commented Apr 27, 2020 via email

@pquentin
Copy link
Member

Merged as part of #1481

@pquentin pquentin closed this Apr 30, 2020
jessecomeau87 pushed a commit to jessecomeau87/Python that referenced this pull request May 20, 2024
…-18475)

The fix for [bpo-39386](https://bugs.python.org/issue39386) attempted to make it so you couldn't reuse a
agen.aclose() coroutine object. It accidentally also prevented you
from calling aclose() at all on an async generator that was already
closed or exhausted. This commit fixes it so we're only blocking the
actually illegal cases, while allowing the legal cases.

The new tests failed before this patch. Also confirmed that this fixes
the test failures we were seeing in Trio with Python dev builds:
  python-trio/trio#1396

https://bugs.python.org/issue39606
(cherry picked from commit 925dc7f)

Co-authored-by: Nathaniel J. Smith <njs@pobox.com>
jessecomeau87 pushed a commit to jessecomeau87/Python that referenced this pull request May 20, 2024
The fix for bpo-39386 attempted to make it so you couldn't reuse a
agen.aclose() coroutine object. It accidentally also prevented you
from calling aclose() at all on an async generator that was already
closed or exhausted. This commit fixes it so we're only blocking the
actually illegal cases, while allowing the legal cases.

The new tests failed before this patch. Also confirmed that this fixes
the test failures we were seeing in Trio with Python dev builds:
  python-trio/trio#1396
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropping python 3.5 support?
3 participants