Remove unnecessary python 3.6 conditionals #20549
Conversation
boring-cyborg
bot
added
area:CLI
area:dev-tools
area:Scheduler
There was a problem hiding this comment.
I think we should only remove it after we release 2.3.0. There was already a case when pyupgrde automatically removed if for 3.6 but the intention for that PR was to chgerry-pick to 2.2.4:
So until we intend to cherry-pick more than just occassinal bugfixes to 2.2.* I think we should keep the "if 3.6" everywhere.
Dockerfile.ci
Outdated
| @@ -25,7 +25,7 @@ ARG AIRFLOW_VERSION="2.2.0.dev0" | |||
| ARG AIRFLOW_IMAGE_REPOSITORY="https://github.com/apache/airflow" | |||
|
|
|||
| # By increasing this number we can do force build of all dependencies | |||
| ARG DEPENDENCIES_EPOCH_NUMBER="6" | |||
There was a problem hiding this comment.
This was bumped because pep562 was removed.
There was a problem hiding this comment.
Actually since pep562 is only installed on python_version < '3.7', I don’t think we even need to rebuild the dependencies because it is never actually installed in any of the images right now.
There was a problem hiding this comment.
Good point, removing this.
|
I'm not opposed to keeping ones that are intended to be cherry-picked back, but why wouldn't we want to remove all the others? And why after 2.3.0, why would we need/want them shipped in that version? We can't remove |
We could do it even now - but only if we accept the risk that things won't work for Python 3.6 when we cherry-pick something to 2.2.4 (or 2.2.5 if we have it). We do not yet know all the stuff we will be cherry-picking (and we can decide tomorrow or in a week that we cherry-pick something new). This is connected with the risk, that during cherry-picking some of those 'if PY36' will be removed by the cherry-pick to 2-2 branch and we will not realize that because the cherry-pick will cleanly apply and our tests might not catch it. As a result we might have failing 2.2.4 on Python 3.6 in some cases. Example: If tomorrow we find an error in Why waiting until 2.3.0 might help ? Because (at least so far) releasing 2.N.0 is the time we stop "mass-cherry-picking" to 2.N-1 branch. So far I think we have not done even once a release for previous minor release once we released 2.N.0. This also decreases the risk of such accidental Python 3.6 removal if we decide to release an urgent bugfix in 2.N-1. (because we will be very selective with cherry-picks). We do not yet know how soon we will release 2.3.0 - it depends on many factors, and we do not know if we will have 2.2.4 or maybe even 2.2.5 and how many cherry-picked commits we will have there - it really depends on whether we will find and fix more important bugs until 2.3.0 is released. So this is really a question of how confident we are that this will not be a problem. I think it's up to you to decide - you are the release manager :D. But you should take the risk into account - and decide consciously to accept the risk. And if you are fine with that I am also fine. I just put "request changes" so that this is not merged accidentally, the case is discussed and decision is made with the risks in mind :). |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
potiuk
removed
the
stale
|
We should be able to merge it already I think |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
@jedcunningham should we merge this before 2.3b1? |
airflow/__init__.py
Outdated
| PY36 = sys.version_info >= (3, 6) | ||
| PY37 = sys.version_info >= (3, 7) |
There was a problem hiding this comment.
I'm actually going to leave these here and in all so we don't break backcompat.
jedcunningham
force-pushed
the
more_36_removal
branch
from
63bdea2 to
6474518
Compare
jedcunningham
removed
the
stale
github-actions
bot
added
the
full tests needed
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
Since Python 3.7 is now the lowest supported version, we no longer need to have conditionals to support 3.6.
jedcunningham
force-pushed
the
more_36_removal
branch
from
6474518 to
f0bde21
Compare
ephraimbuddy
added
the
type:misc/internal
Since Python 3.7 is now the lowest supported version, we no longer need
to have conditionals to support 3.6.