You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "shiavm006 (via GitHub)" <gi...@apache.org> on 2025/12/10 17:19:58 UTC

[PR] Fix BaseXCom.get_all to return empty list instead of None [airflow]

shiavm006 opened a new pull request, #59282:
URL: https://github.com/apache/airflow/pull/59282

   ## Description
   fixes `BaseXCom.get_all()` to return an empty list instead of `None` when no XCom values are found, ensuring the implementation matches the documented behavior.
   the `BaseXCom.get_all()` method's docstring explicitly states:
   > "XComSequenceSliceResult can never have *None* in it, it returns an empty list if no values were found."
   
   However, the implementation returned `None` when `msg.root` was empty, which:
   Contradicted the documented behavior,Violated type system expectations (`XComSequenceSliceResult.root: list[JsonValue]`)
   
   i can be wrong pls tell me the changes if there is something needed.


-- 
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 BaseXCom.get_all to return empty list instead of None [airflow]

Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil closed pull request #59282: Fix BaseXCom.get_all to return empty list instead of None
URL: https://github.com/apache/airflow/pull/59282


-- 
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 BaseXCom.get_all to return empty list instead of None [airflow]

Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #59282:
URL: https://github.com/apache/airflow/pull/59282#issuecomment-3755754370

   Closing due to lack of response from the author


-- 
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 BaseXCom.get_all to return empty list instead of None [airflow]

Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #59282:
URL: https://github.com/apache/airflow/pull/59282#discussion_r2625854984


##########
task-sdk/src/airflow/sdk/bases/xcom.py:
##########
@@ -318,7 +318,7 @@ def get_all(
             raise TypeError(f"Expected XComSequenceSliceResult, received: {type(msg)} {msg}")
 
         if not msg.root:
-            return None
+            return []

Review Comment:
   This is intentional. If you check this part: https://github.com/apache/airflow/blob/main/task-sdk/src/airflow/sdk/execution_time/task_runner.py#L373-L385, we translate empty list (`msg.root` being None) to a None to later filter it properly in task runner. AKA, we translate no xcoms found to be `None` to indicate that no xcoms were found and to keep AF2 behaviour. 
   
   So we cannot make it return `[]`. A better thing to do would be to add a comment here explaining why its done.



-- 
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