ashb commented on code in PR #45075:
URL: https://github.com/apache/airflow/pull/45075#discussion_r1896649073


##########
task_sdk/src/airflow/sdk/api/client.py:
##########
@@ -207,11 +207,16 @@ class XComOperations:
     def __init__(self, client: Client):
         self.client = client
 
-    def get(self, dag_id: str, run_id: str, task_id: str, key: str, map_index: 
int = -1) -> XComResponse:
+    def get(
+        self, dag_id: str, run_id: str, task_id: str, key: str, map_index: int 
| None = -1

Review Comment:
   If this can be none, shouldn't that be the default?



##########
task_sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -111,11 +115,109 @@ def get_template_context(self):
                 "ts_nodash_with_tz": ts_nodash_with_tz,
             }
             context.update(context_from_server)
+        # TODO: We should use/move TypeDict from airflow.utils.context.Context
         return context
 
-    def xcom_pull(self, *args, **kwargs): ...
+    def xcom_pull(
+        self,
+        task_ids: str | None = None,  # TODO: Simplify to a single task_id? 
(breaking change)
+        dag_id: str | None = None,
+        key: str = "return_value",  # TODO: Make this a constant 
(``XCOM_RETURN_KEY``)
+        include_prior_dates: bool = False,  # TODO: Add support for this
+        *,
+        map_indexes: int | None = None,

Review Comment:
   Name says plural, but type only allows one value



##########
task_sdk/src/airflow/sdk/api/client.py:
##########
@@ -207,11 +207,16 @@ class XComOperations:
     def __init__(self, client: Client):
         self.client = client
 
-    def get(self, dag_id: str, run_id: str, task_id: str, key: str, map_index: 
int = -1) -> XComResponse:
+    def get(
+        self, dag_id: str, run_id: str, task_id: str, key: str, map_index: int 
| None = -1
+    ) -> XComResponse:
         """Get a XCom value from the API server."""
         # TODO: check if we need to use map_index as params in the uri
         # ref: 
https://github.com/apache/airflow/blob/v2-10-stable/airflow/api_connexion/openapi/v1.yaml#L1785C1-L1785C81
-        resp = self.client.get(f"xcoms/{dag_id}/{run_id}/{task_id}/{key}", 
params={"map_index": map_index})
+        params = {}
+        if map_index:

Review Comment:
   ```suggestion
           if map_index is not None
   ```
   
   Map index 0 is valid 
   



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

Reply via email to