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

Update Kubernetes library version and backport pickle-fix for Loggers #18797

Merged
merged 1 commit into from
Jan 4, 2022

Conversation

ashb
Copy link
Member

@ashb ashb commented Oct 7, 2021

Previously we pinned this version as v12 as a change to Kube library
internals meant v1.Pod objects now have a logger object inside them, and
couldn't be pickled on Python 3.6.

To fix that we have "backported" the change in Python 3.7 to make Logger
objects be pickled "by name". (In Python 3.7 the change adds
__reduce__ methods on to the Logger and RootLogger objects, but here
we achieve it copyreg stdlib module so we don't monkeypatch
anything.)

This fix is also applied in to airflow core in a separate commit, but we
also apply it here in the provider so that cncf.kubernetes client
library can be updated but still used with older versions of Airflow
that don't have this fix in.

I think we should NOT merge this until after 2.2.0 RC is cut, as I don't think we have time to test the kube client upgrade doesn't break anything before the planned RC release.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@ashb ashb added full tests needed We need to run full set of tests for this PR to merge provider:cncf-kubernetes Kubernetes provider related issues area:dependencies Issues related to dependencies problems labels Oct 7, 2021
@kaxil kaxil added this to the Airflow 2.2.1 milestone Oct 7, 2021
@ashb
Copy link
Member Author

ashb commented Oct 7, 2021

Maybe it is worth splitting this in two actually -- add the reducer for logging to core, but leave the Kube client library version unchanged?

@kaxil
Copy link
Member

kaxil commented Oct 7, 2021

Maybe it is worth splitting this in two actually -- add the reducer for logging to core, but leave the Kube client library version unchanged?

Yeah 2 PRs does make sense.

@ashb
Copy link
Member Author

ashb commented Oct 7, 2021

Split the copyreg in core airflow in to #18798 which is fine to merge in for 2.2.0rc1

@ashb
Copy link
Member Author

ashb commented Oct 12, 2021

Merging now 2.2.0 is out.

@ashb
Copy link
Member Author

ashb commented Oct 12, 2021

Wait -- making sure tests actually run first 😁

@ashb
Copy link
Member Author

ashb commented Nov 12, 2021

Green enough. Merging to mainline for 2.3 -- we shouldn't pull this in to 2.2 series.

@ashb
Copy link
Member Author

ashb commented Nov 12, 2021

Oh wait, the helm tests were cancelled. Those one's probably are important to run. 👀

@ashb ashb requested a review from uranusjr November 12, 2021 14:32
@github-actions github-actions bot closed this Jan 2, 2022
@jedcunningham jedcunningham reopened this Jan 3, 2022
@ashb
Copy link
Member Author

ashb commented Jan 3, 2022

Hmmm helm tests are still timing out here, which means something is not right

@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 4, 2022
Previously we pinned this version as v12 as a change to Kube library
internals meant v1.Pod objects now have a logger object inside them, and
couldn't be pickled on Python 3.6.

To fix that we have "backported" the change in Python 3.7 to make Logger
objects be pickled "by name". (In Python 3.7 the change adds
`__reduce__` methods on to the Logger and RootLogger objects, but here
we achieve it `copyreg` stdlib module so we don't monkeypatch
anything.)

This fix is also applied in to airflow core in a separate commit, but we
also apply it here in the provider so that cncf.kubernetes client
library can be updated but still used with older versions of Airflow
that don't have this fix in.
@ashb
Copy link
Member Author

ashb commented Jan 4, 2022

Better now after a rebase.

@eladkal
Copy link
Contributor

eladkal commented Jan 17, 2022

Removing 2.3.0 milestone as this PR is in the provider release cycle.
Was released in apache-airflow-providers-cncf-kubernetes version 3.0.1

@eladkal eladkal removed this from the Airflow 2.3.0 milestone Jan 17, 2022
@potiuk potiuk added this to the Airflow 2.2.5 milestone Mar 26, 2022
potiuk pushed a commit that referenced this pull request Mar 26, 2022
Previously we pinned this version as v12 as a change to Kube library
internals meant v1.Pod objects now have a logger object inside them, and
couldn't be pickled on Python 3.6.

To fix that we have "backported" the change in Python 3.7 to make Logger
objects be pickled "by name". (In Python 3.7 the change adds
`__reduce__` methods on to the Logger and RootLogger objects, but here
we achieve it `copyreg` stdlib module so we don't monkeypatch
anything.)

This fix is also applied in to airflow core in a separate commit, but we
also apply it here in the provider so that cncf.kubernetes client
library can be updated but still used with older versions of Airflow
that don't have this fix in.

(cherry picked from commit 7222f68)
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Mar 26, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 26, 2022
Previously we pinned this version as v12 as a change to Kube library
internals meant v1.Pod objects now have a logger object inside them, and
couldn't be pickled on Python 3.6.

To fix that we have "backported" the change in Python 3.7 to make Logger
objects be pickled "by name". (In Python 3.7 the change adds
`__reduce__` methods on to the Logger and RootLogger objects, but here
we achieve it `copyreg` stdlib module so we don't monkeypatch
anything.)

This fix is also applied in to airflow core in a separate commit, but we
also apply it here in the provider so that cncf.kubernetes client
library can be updated but still used with older versions of Airflow
that don't have this fix in.

(cherry picked from commit 7222f68)
@ephraimbuddy ephraimbuddy added type:misc/internal Changelog: Misc changes that should appear in change log and removed type:bug-fix Changelog: Bug Fixes labels Mar 26, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 26, 2022
Previously we pinned this version as v12 as a change to Kube library
internals meant v1.Pod objects now have a logger object inside them, and
couldn't be pickled on Python 3.6.

To fix that we have "backported" the change in Python 3.7 to make Logger
objects be pickled "by name". (In Python 3.7 the change adds
`__reduce__` methods on to the Logger and RootLogger objects, but here
we achieve it `copyreg` stdlib module so we don't monkeypatch
anything.)

This fix is also applied in to airflow core in a separate commit, but we
also apply it here in the provider so that cncf.kubernetes client
library can be updated but still used with older versions of Airflow
that don't have this fix in.

(cherry picked from commit 7222f68)
potiuk pushed a commit that referenced this pull request Mar 28, 2022
@ephraimbuddy ephraimbuddy removed this from the Airflow 2.2.5 milestone Mar 28, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 28, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dependencies Issues related to dependencies problems area:providers full tests needed We need to run full set of tests for this PR to merge provider:cncf-kubernetes Kubernetes provider related issues type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants