Re: [PR] Record OpenAI Responses token usage and response id in XCom [airflow]

["kaxil (via GitHub)" <gi...@apache.org>]

kaxil commented on code in PR #72151:
URL: https://github.com/apache/airflow/pull/72151#discussion_r3957929735


##########
providers/openai/src/airflow/providers/openai/operators/openai.py:
##########
@@ -99,6 +101,12 @@ class OpenAIResponseOperator(BaseOperator):
         :ref:`howto/operator:OpenAIResponseOperator`
         For possible options, see:
         https://platform.openai.com/docs/api-reference/responses/create
+
+    ``execute`` also pushes two XCom keys: ``response_id`` (the response's ID, usable as

Review Comment:
   This lands in the docstring, but `providers/openai/docs/operators/openai.rst:47` still says the operator just "returns the response's aggregated output text", and the example DAG doesn't show the chaining this PR is for. Could you add the two keys to the howto and an `xcom_pull(key="response_id")` -> `previous_response_id` step to `example_openai.py`? The rst is where people look first.



##########
providers/openai/src/airflow/providers/openai/hooks/openai.py:
##########
@@ -272,6 +272,23 @@ def create_response(self, input: Any, model: str = "gpt-4o-mini", **kwargs: Any)
         """
         return self.conn.responses.create(model=model, input=input, **kwargs)
 
+    @staticmethod
+    def summarize_response_usage(response: Response) -> dict[str, Any] | None:

Review Comment:
   Is this meant as hook-user API, or just a helper for the operator? It never touches `self.conn`, and once released it's public surface the provider owes back-compat on for a one-line `model_dump`. If it is for hook users, the Responses list in `openai.rst` should name it alongside `create_response`/`get_response`; if not, inlining it in `execute` keeps the hook to connection-backed calls.



##########
providers/openai/src/airflow/providers/openai/operators/openai.py:
##########
@@ -131,6 +139,8 @@ def execute(self, context: Context) -> str:
                 response.status,
             )
         self.log.info("Generated response %s", response.id)
+        context["ti"].xcom_push(key="response_id", value=response.id)

Review Comment:
   `do_xcom_push=False` won't suppress these two. That flag only gates the return-value push in the task runner, so someone who turns it off to keep the metadata DB lean still gets two XCom rows per run. Amazon gates its extra id pushes on it (`emr.py:196`, `ecs.py:545`, `eventbridge.py:83`), and adding the guard now is cheaper than changing the behaviour after this ships.



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