amoghrajesh commented on code in PR #68848:
URL: https://github.com/apache/airflow/pull/68848#discussion_r3457147481
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/backcompat/test_backwards_compat_converters.py:
##########
@@ -102,6 +102,35 @@ def test_convert_from_dict_with_invalid_type():
assert str(exc_info.value) == "Expected dict or <class
'unittest.mock.Mock'>, got <class 'str'>"
+def test_convert_from_dict_is_picklable_in_cluster(monkeypatch):
+ """A model deserialized from a dict must not capture the unpicklable
in-cluster Configuration.
+
+ In-cluster, the kubernetes client installs a process-global default
``Configuration`` whose
+ ``refresh_api_key_hook`` is an unpicklable local closure.
``_convert_from_dict`` must deserialize
+ through a fresh ``Configuration`` so the model (and every nested object)
stays picklable.
+ """
+ import pickle
+
+ from kubernetes.client import Configuration
Review Comment:
Top level import pls.
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:
##########
@@ -36,7 +36,10 @@ def _convert_from_dict(obj, new_class):
if isinstance(obj, new_class):
return obj
if isinstance(obj, dict):
- api_client = ApiClient()
+ # A fresh Configuration() keeps the deserialized model picklable:
in-cluster, the kubernetes
+ # client (v36) would otherwise stamp the global Configuration, whose
refresh_api_key_hook is
+ # an unpicklable local closure, onto the model and every nested object.
Review Comment:
This same comment seems to repeat 3 times - in here, in job.py and in
pod_generator.py, one place is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
