jason810496 commented on code in PR #59129:
URL: https://github.com/apache/airflow/pull/59129#discussion_r2594915905
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_xcom.py:
##########
@@ -683,11 +682,50 @@ def test_patch_xcom_entry(self, key, patch_body,
expected_status, expected_detai
assert response.status_code == expected_status
if expected_status == 200:
- assert response.json()["value"] ==
XComModel.serialize_value(new_value)
+ # The returned value should be serialized once (stored format in
DB)
+ expected_value = XComModel.serialize_value(patch_body["value"])
+ assert response.json()["value"] == expected_value
else:
assert response.json()["detail"] == expected_detail
check_last_log(session, dag_id=TEST_DAG_ID, event="update_xcom_entry",
logical_date=None)
+ def test_patch_xcom_entry_value_usability(self, test_client, session):
+ """Test that patched XCom values can be retrieved and used correctly
without excessive serialization."""
+ # Create initial XCom
+ self._create_xcom(TEST_XCOM_KEY, 1)
Review Comment:
Would it be better to test the nested JSON structure instead of simple
integer in order to verify the serialization behavior?
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/xcom.py:
##########
@@ -335,7 +333,7 @@ def update_xcom_entry(
f"The XCom with key: `{xcom_key}` with mentioned task instance
doesn't exist.",
)
- # Update XCom entry
- xcom_entry.value = json.dumps(xcom_new_value)
+ # Update XCom entry - serialize the value only once
+ xcom_entry.value = XComModel.serialize_value(patch_body.value)
Review Comment:
So actually it seems that the problem of #59032 might be double
serialization at line 317.
--
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]
