Skip to content

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

Merged
ashb merged 1 commit into
apache:mainfrom
astronomer:update-kube-client
Jan 4, 2022
Merged

Update Kubernetes library version and backport pickle-fix for Loggers#18797
ashb merged 1 commit into
apache:mainfrom
astronomer:update-kube-client

Conversation

@ashb

@ashbashb commented Oct 7, 2021

Copy link
Copy Markdown
Member

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.

@ashbashb added full tests needed We need to run full set of tests for this PR to merge provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:dependencies Issues related to dependencies problems labels Oct 7, 2021
@kaxilkaxil added this to the Airflow 2.2.1 milestone Oct 7, 2021
@ashb

ashb commented Oct 7, 2021

Copy link
Copy Markdown
MemberAuthor

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

kaxil commented Oct 7, 2021

Copy link
Copy Markdown
Member

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

ashb commented Oct 7, 2021

Copy link
Copy Markdown
MemberAuthor

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

@ashb
ashbforce-pushed the update-kube-client branch from 2503c53 to 30b37a8CompareOctober 7, 2021 09:43
@ashb

ashb commented Oct 12, 2021

Copy link
Copy Markdown
MemberAuthor

Merging now 2.2.0 is out.

@ashb

ashb commented Oct 12, 2021

Copy link
Copy Markdown
MemberAuthor

Wait -- making sure tests actually run first 😁

@ashb
ashbforce-pushed the update-kube-client branch from 30b37a8 to 9eb6ebeCompareOctober 12, 2021 13:53
@ashbashb modified the milestones: Airflow 2.2.1, Airflow 2.2.2Oct 14, 2021
@ashb
ashbforce-pushed the update-kube-client branch from 9eb6ebe to c2028deCompareNovember 1, 2021 18:19
@ashb
ashbforce-pushed the update-kube-client branch 2 times, most recently from 2c2c555 to 77661b7CompareNovember 11, 2021 10:50
@ashb

ashb commented Nov 12, 2021

Copy link
Copy Markdown
MemberAuthor

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

@ashb

ashb commented Nov 12, 2021

Copy link
Copy Markdown
MemberAuthor

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

Comment threadtests/kubernetes/test_client.py Outdated
@ashb
ashb requested a review from uranusjrNovember 12, 2021 14:32
@ashb

ashb commented Jan 3, 2022

Copy link
Copy Markdown
MemberAuthor

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

@github-actionsgithub-actionsBot 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
ashbforce-pushed the update-kube-client branch from 0fa2c58 to 0d1dc45CompareJanuary 4, 2022 10:57
@ashb

ashb commented Jan 4, 2022

Copy link
Copy Markdown
MemberAuthor

Better now after a rebase.

@eladkal

Copy link
Copy Markdown
Contributor

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

@eladkaleladkal removed this from the Airflow 2.3.0 milestone Jan 17, 2022
@potiukpotiuk 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)
@ephraimbuddyephraimbuddy 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)
@ephraimbuddyephraimbuddy 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
@ephraimbuddyephraimbuddy 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dependenciesIssues related to dependencies problemsarea:providersfull tests neededWe need to run full set of tests for this PR to mergeprovider:cncf-kubernetesKubernetes (k8s) provider related issuestype:misc/internalChangelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@ashb@kaxil@eladkal@potiuk@uranusjr@jedcunningham@ephraimbuddy