You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Arunodoy18 (via GitHub)" <gi...@apache.org> on 2025/12/06 09:15:11 UTC
[PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Arunodoy18 opened a new pull request, #59129:
URL: https://github.com/apache/airflow/pull/59129
The update_xcom_entry endpoint was calling XComModel.serialize_value twice, causing values to be excessively serialized. This made the updated XCom values unusable when retrieved by tasks.
Changes:
- Removed duplicate XComModel.serialize_value call in update_xcom_entry
- Fixed existing test to remove double serialization in assertion
- Added comprehensive test to verify patched values are correctly serialized and can be retrieved/deserialized properly
Fixes: apache/airflow#44456
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #59129:
URL: https://github.com/apache/airflow/pull/59129#issuecomment-3633249076
Needs rebase + conflict solving.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "Arunodoy18 (via GitHub)" <gi...@apache.org>.
Arunodoy18 commented on PR #59129:
URL: https://github.com/apache/airflow/pull/59129#issuecomment-3713455093
Thanks for the direct feedback. I understand the concerns around scope, testing, and unrelated changes. I’ll slow down and focus on smaller, well-scoped contributions aligned with prior discussion.
________________________________
From: Jarek Potiuk ***@***.***>
Sent: 06 January 2026 01:05
To: apache/airflow ***@***.***>
Cc: Arunodoy Banerjee ***@***.***>; Mention ***@***.***>
Subject: Re: [apache/airflow] Fix double serialization in XCom PATCH API endpoint (PR #59129)
[https://avatars.githubusercontent.com/u/595491?s=20&v=4]potiuk left a comment (apache/airflow#59129)<https://github.com/apache/airflow/pull/59129#issuecomment-3711821972>
Also @Arunodoy18<https://github.com/Arunodoy18> - one comment from me. Opening such PRs is NOT increasing your value as developer and does not bring you any benefits. It mostly makes your history showing that you are not really reliable author of PRs - it will take long time (if possible) to rebuild your reputation.
—
Reply to this email directly, view it on GitHub<https://github.com/apache/airflow/pull/59129#issuecomment-3711821972>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BOEIQ34TOWO6E43BPJTJDVT4FK4HTAVCNFSM6AAAAACOHKSV3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMJRHAZDCOJXGI>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59129:
URL: https://github.com/apache/airflow/pull/59129#issuecomment-3711821972
Also @Arunodoy18 - one comment from me. Opening such PRs is NOT increasing your value as developer and does not bring you any benefits. It mostly makes your history showing that you are not really reliable author of PRs - it will take long time (if possible) to rebuild your reputation.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "Arunodoy18 (via GitHub)" <gi...@apache.org>.
Arunodoy18 commented on PR #59129:
URL: https://github.com/apache/airflow/pull/59129#issuecomment-3619801088
I have solved the issue , please assure reviewing the issue which is solved, If any thing is needed which is to be changed , please tell. Thank you .
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59129:
URL: https://github.com/apache/airflow/pull/59129#discussion_r2625982218
##########
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:
Lets use a tuple instead? That would ensure the ser/deser works fine.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #59129:
URL: https://github.com/apache/airflow/pull/59129#issuecomment-3711743113
@Arunodoy18 I am going to close your PRs -- Please review and test your changes with correct PR description. Using LLMs without those increase maintenance burdens and CI run time.
Feel free to recreate focussed PRs following those guidelines.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix double serialization in XCom PATCH API endpoint [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil closed pull request #59129: Fix double serialization in XCom PATCH API endpoint
URL: https://github.com/apache/airflow/pull/59129
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org