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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
