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

Serialize pod_override to JSON before pickling executor_config #24356

Conversation

dstandish
Copy link
Contributor

If we unpickle a k8s object that was pickled under an earlier k8s library version, then the unpickled object may throw an error when to_dict is called. To be more tolerant of version changes we convert to JSON using Airflow's serializer before pickling.

@dstandish dstandish force-pushed the serialize-pod_override-before-pickle-executor-config branch from 8a6a0a5 to 3a2dbeb Compare June 9, 2022 18:45
@ashb
Copy link
Member

ashb commented Jun 9, 2022

Not sure if this impacts things: but executor config can be used for other executors too

@dstandish
Copy link
Contributor Author

Yeah I tried to be mindful of that. I don't just serialize the whole executor config. I only serialize the pod_override node if the config is a dict and pod_override is a V1Pod. So that seems like it should be selective enough, if a bit hacky.

@dstandish dstandish force-pushed the serialize-pod_override-before-pickle-executor-config branch from 3a2dbeb to 9f1a16c Compare June 17, 2022 16:17
@dstandish dstandish marked this pull request as ready for review June 17, 2022 17:25
@dstandish dstandish force-pushed the serialize-pod_override-before-pickle-executor-config branch 3 times, most recently from e21c922 to 769efd9 Compare June 17, 2022 21:01
@github-actions
Copy link

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.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 22, 2022
If we unpickle a k8s object that was pickled under an earlier k8s library version, then the unpickled object may throw an error when to_dict is called.  To be more tolerant of version changes we convert to JSON using Airflow's serializer before pickling.
@dstandish dstandish force-pushed the serialize-pod_override-before-pickle-executor-config branch from 258a90a to 6255d97 Compare June 25, 2022 03:57
@potiuk
Copy link
Member

potiuk commented Jul 4, 2022

Random failure. Merging.

@potiuk potiuk merged commit c1d621c into apache:main Jul 4, 2022
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Aug 12, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.3.4 milestone Aug 19, 2022
ephraimbuddy pushed a commit that referenced this pull request Aug 19, 2022
* Serialize pod_override to JSON before pickling executor_config

If we unpickle a k8s object that was pickled under an earlier k8s library version, then the unpickled object may throw an error when to_dict is called.  To be more tolerant of version changes we convert to JSON using Airflow's serializer before pickling.

(cherry picked from commit c1d621c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants