dstandish commented on a change in pull request #19505:
URL: https://github.com/apache/airflow/pull/19505#discussion_r808550810
##########
File path: airflow/models/xcom.py
##########
@@ -458,8 +469,8 @@ def clear(
return query.delete()
@staticmethod
- def serialize_value(value: Any):
- """Serialize Xcom value to str or pickled object"""
+ def serialize_value(value: Any, key=None, task_id=None, dag_id=None,
execution_date=None, run_id=None):
Review comment:
hmm... yeah it does break some tests to make `value` keyword-only
```
tests/models/test_xcom.py::TestXCom::test_resolve_xcom_class_fallback_to_basexcom:
TypeError: serialize_value() takes 0 positional arguments but 1 was given
tests/models/test_xcom.py::TestXCom::test_resolve_xcom_class_fallback_to_basexcom_no_config:
TypeError: serialize_value() takes 0 positional arguments but 1 was given
Error: Process completed with exit code 1.
```
e.g. here's one test:
```python
@conf_vars({("core", "xcom_backend"): "", ("core",
"enable_xcom_pickling"): "False"})
def test_resolve_xcom_class_fallback_to_basexcom(self):
cls = resolve_xcom_backend()
assert issubclass(cls, BaseXCom)
assert cls().serialize_value([1]) == b"[1]"
```
it seems unlikely that a user would be relying on this. but it's possible.
for now i've updated it to keep `value` as positional -- but please do let me
know your thoughhts.
--
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]
