amoghrajesh commented on code in PR #68848:
URL: https://github.com/apache/airflow/pull/68848#discussion_r3458431794
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:
##########
@@ -36,7 +36,8 @@ 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 picklabled
Review Comment:
```suggestion
# A fresh Configuration() keeps the deserialized model picklable
```
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/backcompat/test_backwards_compat_converters.py:
##########
@@ -102,6 +103,32 @@ 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.
+ """
+
+ def _make_unpicklable_hook():
+ def _refresh_api_key(config):
+ return None
+
+ return _refresh_api_key
+
Review Comment:
Is there a need for a wrapper function here? Can't we just do:
```python
def _refresh_api_key(config):
return None
dirty.refresh_api_key_hook = _refresh_api_key
```
--
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]
