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

Technical aspects related to Python 2.7 and 3.4 support (pytest 5.0) #5275

Open
nicoddemus opened this issue May 16, 2019 · 38 comments

Comments

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented May 16, 2019

This issue is to discuss the technical aspects of pytest dropping Python 2 and 3.4 in pytest 5.0 and long term support of the 4.6 series.

For a more user-centric point of view, please see Python 2.7 and 3.4 support plan in the docs.


(This post has since then edited from the original version after the discussion that took place here).

Unless decided otherwise, the last pytest version to support Python 2.7 and 3.4 will be the 4.5 series, which is the last minor release on PyPI.

This issue contains the agreed process on how we will proceed with the pytest 5.0 release and long term support plans for the 4.5 series.

What goes into 4.6.X releases

New 4.6.X releases will contain bug fixes only:

  • A bug will be fixed/ported if, in a normal release, we would merge it in the master branch: often we apply bug fixes to the features branch because even though we are fixing a bug, it may break test suites, so we often take the safest route. Those bugs are often not critical, and won't be ported to the maintenance branch.
  • No new features will be added on this branch, however small.

Who will handle porting bugs

After a PR for a bug fix is merged, and the bug is applicable to the 4.6.X series according to the rules stated previously:

We expect core maintainers who provided the initial bug fix to also create a new PR porting the bug.

From other contributors we expect core maintainers to step up to create a new PR porting the bug if the original contributor is not interested in doing so. @nicoddemus and @asottile applied to be the forefront here, but will gladly accept help from other core maintainers.

When will 4.6.X releases happen

New 4.6.X releases will happen after we have a few bugs in place to release, or if a few weeks have passed (say a single bug has been fixed a month after the latest 4.5.X release). No hard rules here (as with any other pytest release really), just a ballpark.

Backporting changes into the 4.6 release

  1. git fetch --all --prune
  2. git checkout origin/4.6-maintenance -b backport-XXXX # use the PR number here
  3. locate the merge commit on master with git log
  4. git cherry-pick -m1 REVISION # use the revision you found in 3.
  5. open a PR targeting 4.6-maintenance: prefix the message with [4.6] so it is an obvious backport, delete the PR body (it usually contains a duplicated commit message)

Note for backport PRs a review is not necessary -- though if it it falls into one of these categories a review is strongly suggested:

  • The cherry-pick did not apply cleanly
  • This is a bugfix for only the 4.6.x series

Steps for the first 5.0 release

  1. The absolute first step is to mark features as Python 3 only: ✔️
    • Update setup.py classifiers and python_requires to declare pytest only supports Python 3.5+.
    • Drop Python<3.5 from CI.
    • Update the "Python 2.7 and 3.4 support plan".
  2. After 4.6 is released, we push a pytest4-maintenance branch pointing to 4.6.0 initially. ✔️
  3. The next step is to run pyupgrade --py3-plus (I guess @asottile plans to volunteer here 😉); ✔️
  4. Turn warnings into errors. ✔️
  5. Release 5.0. 🎉

When will we start the 5.0 release process

It is my opinion that we can proceed with the steps above right away, as we are quickly approaching end of May.

If nobody opposes, I plan to start with the first step this week or early next week.

@nicoddemus nicoddemus pinned this issue May 16, 2019
@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 16, 2019

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 16, 2019

Hmmm, if we're not going to go full-py3 on the main branches I don't see much benefit of dropping python2 at all (we'd be still maintaining python2 support but with nothing checking it and extra workflow and process). Yes it's going to be potentially ~slightly more work to backport fixes but that's already going to be the case when refactors due to new features etc. happen. I think we should burn the bridges on master / features.

@Zac-HD

This comment has been minimized.

Copy link
Member

@Zac-HD Zac-HD commented May 17, 2019

I'm with @asottile on this one, and would fully exploit Py3 on all the main branches.

Basically, I think the time we save backporting will be dwarfed by the productivity gains from feature development, which is after all much more common! Even just reducing the volume of code is a win 😄

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented May 17, 2019

For the actual release I believe only declaring py3only is the practical way

Then the 5 series can gradually sort out the ckeanups.

This ensures we can do a 4.x release at any point before 5 and it ensures a time fame of gradually more expensive backports

This path is mainly for risk mitigation.

I can understand the desire to konmari the codebase asap

Let's ensure that process is wrapped so the categories of cleanup happen consistently and spread a bit over time

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 17, 2019

it's a snap to just run pyupgrade --py3-plus and immediately reap the benefits -- why gradual?

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented May 17, 2019

I'm fine with doing that as long as we have a very clear cut line on who pays the backports costs in the core support time frame and ensure they happen in a timely fashion

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 17, 2019

@RonnyPfannschmidt's point is the reason why I propose we don't do major changes: we have committed to port bug fixes to the 4.x maintenance branch until the end of the year. This is a commitment that we maintainers have to address.

To that end, we will have to:

  1. For each bug-fix submitted by a core maintainer, it would also be required another PR to the 4.x maintenance branch by the same core maintainer, as a general rule.

  2. For bug-fixes from external contributors, one of the core maintainers has to step up (probably we should assign this in a round robin fashion) to port the same bug fix to the 4.x maintenance branch.

So this maintenance process will be that much harder if we go "all-in" Python 3, as this will make just cherry-picking a change into the maintenance branch impossible; gradual changes might allow us to cherry-pick and solve an eventual conflict more easily.

As @RonnyPfannschmidt mentions, if we go all-in with Python 3, we need a clear commitment from the core maintainers about ensuring the workflow above happens.


We can discuss a slight change of plans as well.

  1. We keep Python 2/3 support until the end of the year. This has the advantage of not introducing additional maintenance burdens to us, but will prevent us from using Python 3 features in the code base until then. This is a bummer of course, I also would like to use Python 3 only as much as the next guy.

  2. We drop Python 2 completely now, and only keep a 4.x maintenance branch in case users want to back port critical bug-fixes themselves. This was the option I wanted in the first place when we started this discussion, but now it is too late as we've committed support for Python 2 until the end of the year for our users. I only mention this in passing, I don't think we can do this anymore at this point.

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented May 17, 2019

will be that much harder if we go "all-in" Python 3, as this will make just cherry-picking a change into the maintenance branch impossible; gradual changes might allow us to cherry-pick and solve an eventual conflict more easily.

I do not think cherry-picking will be more complicated - it would e.g. not remove six being imported etc, and should only have an impact really if it touches code that has been made py3-only directly.

it's a snap to just run pyupgrade --py3-plus

Maybe we could have branch/PR already to see how this would look like?

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 17, 2019

I do not think cherry-picking will be more complicated - it would e.g. not remove six being imported etc, and should only have an impact really if it touches code that has been made py3-only directly.

I agree it might not be problematic, but again it might depending on the changes we do.


After reading through this, the most important part of this movement IMO is that everyone is on-board with this:

we need a clear commitment from the core maintainers about ensuring the workflow above happens.

What I would not like to see is bug-fixes not being ported because we made things more difficult by going full Python 3.

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 17, 2019

Maybe we could have branch/PR already to see how this would look like?

Good idea! 👍

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 17, 2019

here's a demo branch -- I split up the automated / manual steps that I'd take: https://github.com/asottile/pytest/pull/new/pyupgrade_demo

I used the following tools:

  • pyupgrade (run with --py3-plus since we still are planning to support py35, I removed --keep-percent-format since we no longer care about the py2-unicode behaviour here)
  • reorder-python-imports (run with --py3-plus)
  • fix-encoding-pragma (run with --remove)
  • then mostly some grep / flake8

I'm willing to take up the backport cost, I'd even take up the burden to run all of the backport PRs if it means I can use python3 on the main branch

@Zac-HD

This comment has been minimized.

Copy link
Member

@Zac-HD Zac-HD commented May 20, 2019

🎉

My only possible quibble is that there are a few places where pyupgrade took u"%s" % ... and just stripped the prefix, without converting to the .format method. I assume running it again would reach the fixpoint, but you might want to fiddle with the internals a bit.

Other things to check: auditing git grep sys.version_info to find remaining Py2-specific code, as I suspect most of that can't be automatically stripped (by current tools).

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 20, 2019

'fraid that one's intentional: asottile/pyupgrade#110 (comment)

pyupgrade tries really hard to not auto-break your code and so it avoids rewrites that could change behaviour

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented May 20, 2019

I removed --keep-percent-format since we no longer care about the py2-unicode behaviour here

I think it is better not touching/changing those unnecessarily, i.e. I'd rather keep them to minimize the diff.
The same (minimize diff) also applies to the "u" prefix.

But I think it is good in general after a quick review.

Would be good to open a PR with it already, to get a coverage report.

And https://github.com/pytest-dev/pytest/compare/master...asottile:pyupgrade_demo?expand=1#diff-4ec37b1ad5b7d41fdac56c907d08dde1R441 (lru_cache) can be removed from compat.

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 20, 2019

I removed --keep-percent-format since we no longer care about the py2-unicode behaviour here

I think it is better not touching/changing those unnecessarily, i.e. I'd rather keep them to minimize the diff.
The same (minimize diff) also applies to the "u" prefix.

If we're going to do python2 / python3 split at all -- there's no reason to minimize the diff. With that attitude we can't take advantage of new features and we can't build new features and we can't refactor as those all maximize the diff.

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented May 20, 2019

If we're going to do python2 / python3 split at all -- there's no reason to minimize the diff.

Isn't "avoiding merge/cherry-pick conflicts" a good reason?

It does not mean to not cause a diff when making use of new features, but using str.format() instead of percent-format is not really a "feature" to me in this regard (rather a question of style).
Also there is no harm in keeping object with class definitions - although this is rather unlikely to cause conflicts. I can also see that conflicts are trivial to solve in all cases, but would still rather not have them in the first place, since it prevents automatic backports etc.

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 20, 2019

to me it sounds like you're saying "don't improve the code because it might cause conflicts", the cost of an automatic vs manual backport is very very small. If we're going to do this at all, I'd like us to fully embrace moving forward without fear, with the understanding that conflicts are a natural part of maintaining a fork

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 20, 2019

If we're going to do this at all, I'd like us to fully embrace moving forward without fear, with the understanding that conflicts are a natural part of maintaining a fork

I kind of agree. Specially after this very encouraging comment:

I'm willing to take up the backport cost, I'd even take up the burden to run all of the backport PRs if it means I can use python3 on the main branch

I also step in here to help with the ports, I don't think we should burden a single person with this. 😁

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented May 20, 2019

hi, since my deeper involvement is not yet clear, i am delighted to see a reasonable bus factor being set up

i understand that there is a natural desire to avoid of the potential of having to shoulder things one didn't commit to oneself (and more back-porting conflicts are certainly one of those things where one really shouldn't have to shoulder other peoples commitments)

i believe with both @asottile and @nicoddemus committing to support the backporting efforts it its not unreasonable to go for the better code now option

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 22, 2019

So are we in agreement with moving this forward "soon", as we are nearing the end of May already?

@blueyed, you were of the opinion that we should not burn bridges at this point, after the discussion that followed do you still feel the same way?

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented May 22, 2019

@nicoddemus
We're not burning bridges here, but just make it more likely to cause conficts with backporting - but that is not critical.
I still think that percent-format could be just kept where it is used already for example. I still prefer it myself often - but as you can see this is about style mostly.

So I am +0 on this.

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 23, 2019

We're not burning bridges here, but just make it more likely to cause conficts with backporting - but that is not critical.

Sorry, just my way of saying: "we are going full Python 3, even if that means more conflicts when porting bugs to the maintenance branch".

Given that IMO we are on the same page, I will update the original post then.

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 23, 2019

Hi, updated the original post, please review it and let me know if you disagree with anything.

One point that we might go differently than what I wrote: we could make 4.6.0 release with what we have on features currently, and make 4.6.0 the last release to support Python 2.7 and 3.4. This way Python 2 users will have access to what we have on features now.

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented May 23, 2019

looks good, let's do a 4.6.0 release since it's essentially free for us to do so :)

@Zac-HD

This comment has been minimized.

Copy link
Member

@Zac-HD Zac-HD commented May 23, 2019

No particular objection to a 4.6, but it doesn't look like we have anything worth releasing on features at the moment: https://github.com/pytest-dev/pytest/compare/features

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 23, 2019

No particular objection to a 4.6, but it doesn't look like we have anything worth releasing on features at the moment:

Ahh right, those are only bug-fixes that we have on master already. 👍

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 23, 2019

So I propose we do a 4.5.1 in the next days and then move to release 5.0. 👍

@nicoddemus nicoddemus unpinned this issue May 23, 2019
@nicoddemus nicoddemus pinned this issue May 23, 2019
@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented May 27, 2019

I propose a change of plans: release 4.6.0 this week still supporing Python 2.7 and 3.4, as some features have made into features and a few others should be merged soon, and then proceed with this right after, probably right after the release itself. This also has the advantage that we can officially announce that 4.6 will be the last version supporting Python 2.7 and 3.4.

nicoddemus added a commit to nicoddemus/pytest that referenced this issue May 27, 2019
As discussed in pytest-dev#5275
Sup3rGeo added a commit to Sup3rGeo/pytest that referenced this issue May 30, 2019
@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented Jun 1, 2019

I've created the 4.6-maintenance branch and applied branch protection to it!

Would it be useful to create some labels to better track backporting [needs-backport] (TODO: backport) and [backported] (backported)?

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented Jun 1, 2019

We could also use (a) milestone(s) for tracking backports, but it would not work well with closed PRs/issues then.
i think it would be better to create the backport PR right away after merging instead.

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented Jun 1, 2019

Would it be useful to create some labels to better track backporting [needs-backport]

i think it would be better to create the backport PR right away after merging instead.

Agree with both 😁

I think we should create a label needs-backport for PRs, to make it explicit that a given PR should be backported to the maintenance branch. I also agree that we should create the backport right away preferably. One approach doesn't exclude the other.

In fact we can try this right away with #5356. 👍

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 2, 2019
* Update setup.py requires and classifiers
* Drop Python 2.7 and 3.4 from CI
* Update docs dropping 2.7 and 3.4 support
* Fix mock imports and remove tests related to pypi's mock module
* Add py27 and 34 support docs to the sidebar
* Remove usage of six from tmpdir
* Remove six.PY* code blocks
* Remove sys.version_info related code
* Cleanup compat
* Remove obsolete safe_str
* Remove obsolete __unicode__ methods
* Remove compat.PY35 and compat.PY36: not really needed anymore
* Remove unused UNICODE_TYPES
* Remove Jython specific code
* Remove some Python 2 references from docs

Related to pytest-dev#5275
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 2, 2019
* Update setup.py requires and classifiers
* Drop Python 2.7 and 3.4 from CI
* Update docs dropping 2.7 and 3.4 support
* Fix mock imports and remove tests related to pypi's mock module
* Add py27 and 34 support docs to the sidebar
* Remove usage of six from tmpdir
* Remove six.PY* code blocks
* Remove sys.version_info related code
* Cleanup compat
* Remove obsolete safe_str
* Remove obsolete __unicode__ methods
* Remove compat.PY35 and compat.PY36: not really needed anymore
* Remove unused UNICODE_TYPES
* Remove Jython specific code
* Remove some Python 2 references from docs

Related to pytest-dev#5275
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 2, 2019
* Update setup.py requires and classifiers
* Drop Python 2.7 and 3.4 from CI
* Update docs dropping 2.7 and 3.4 support
* Fix mock imports and remove tests related to pypi's mock module
* Add py27 and 34 support docs to the sidebar
* Remove usage of six from tmpdir
* Remove six.PY* code blocks
* Remove sys.version_info related code
* Cleanup compat
* Remove obsolete safe_str
* Remove obsolete __unicode__ methods
* Remove compat.PY35 and compat.PY36: not really needed anymore
* Remove unused UNICODE_TYPES
* Remove Jython specific code
* Remove some Python 2 references from docs

Related to pytest-dev#5275
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 2, 2019
* Update setup.py requires and classifiers
* Drop Python 2.7 and 3.4 from CI
* Update docs dropping 2.7 and 3.4 support
* Fix mock imports and remove tests related to pypi's mock module
* Add py27 and 34 support docs to the sidebar
* Remove usage of six from tmpdir
* Remove six.PY* code blocks
* Remove sys.version_info related code
* Cleanup compat
* Remove obsolete safe_str
* Remove obsolete __unicode__ methods
* Remove compat.PY35 and compat.PY36: not really needed anymore
* Remove unused UNICODE_TYPES
* Remove Jython specific code
* Remove some Python 2 references from docs

Related to pytest-dev#5275
@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented Jun 2, 2019

Do we have a good place to document the backporting procedure or is here as good as any?

I've found this works the best (for now):

  1. git fetch --all --prune
  2. git checkout origin/4.6-maintenance -b backport_XXXX # use the PR number here
  3. locate the merge comment on master with git log
  4. git cherry-pick -m1 REVISION # use the revision you found in 3.
  5. open a PR targetting 4.6-maintenance # prefix the message with [4.6] so it is an obvious backport, delete the PR body (it usually contains a duplicated commit message)
@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented Jun 2, 2019

I'm going to remove "Requires code review" for the 4.6-maintenance branch -- please use your best judgement for backports (if the patch applies cleanly, probably doesn't need another review)

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented Jun 2, 2019

Do we have a good place to document the backporting procedure or is here as good as any?

How about editing the original post to include those commands?

please use your best judgement for backports (if the patch applies cleanly, probably doesn't need another review)

sgtm 👍

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 2, 2019
* Update setup.py requires and classifiers
* Drop Python 2.7 and 3.4 from CI
* Update docs dropping 2.7 and 3.4 support
* Fix mock imports and remove tests related to pypi's mock module
* Add py27 and 34 support docs to the sidebar
* Remove usage of six from tmpdir
* Remove six.PY* code blocks
* Remove sys.version_info related code
* Cleanup compat
* Remove obsolete safe_str
* Remove obsolete __unicode__ methods
* Remove compat.PY35 and compat.PY36: not really needed anymore
* Remove unused UNICODE_TYPES
* Remove Jython specific code
* Remove some Python 2 references from docs

Related to pytest-dev#5275
@hugovk hugovk referenced this issue Jun 4, 2019
0 of 6 tasks complete
@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented Jun 5, 2019

Just a (too late) thought: couldn't we have used master instead of 4.6-maintenance, until 4.7 is released?
I.e. everything going to master now also gets backported effectively, no?

But given the current situation we could just skip features until the next release then, yes?

@asottile

This comment has been minimized.

Copy link
Member

@asottile asottile commented Jun 5, 2019

yeah too late now

@blueyed

This comment has been minimized.

Copy link
Contributor

@blueyed blueyed commented Jun 5, 2019

But given the current situation we could just skip features until the next release then, yes?

features is not ahead of master, so let's do this then until 4.7?

But also not really / strictly necessary, especially for PRs that might live longer.. so let's rather continue as usual.

@nicoddemus

This comment has been minimized.

Copy link
Member Author

@nicoddemus nicoddemus commented Jun 5, 2019

Just a (too late) thought: couldn't we have used master instead of 4.6-maintenance, until 4.7 is released?

Actually we don't plan to have a 4.7 release at all, only 5.0. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.