Bump up openai version to >=1.0 & use get_conn #36014
Conversation
pankajkoti
force-pushed
the
openai-get-conn
branch
from
329d3f4 to
2cdfcfd
Compare
pankajkoti
requested review from
utkarsharma2,
Lee-W,
ephraimbuddy,
sunank200 and
phanikumv
pankajkoti
force-pushed
the
openai-get-conn
branch
from
7dcc4d6 to
556601d
Compare
pankajkoti
added
the
use public runners
| return OpenAI( | ||
| api_key=password, | ||
| base_url=url, | ||
| ) |
There was a problem hiding this comment.
We should expand this to take other args through the extra. See https://github.com/openai/openai-python/blob/e36956673d9049713c91bca6ce7aebe58638f483/src/openai/_client.py#L65-L82
There was a problem hiding this comment.
Thanks. Accepting those as extras kawrgs so that we don't have to keep updating the kwargs as the client changes across versions.
Updated the connections doc with an example for the same.
pankajkoti
force-pushed
the
openai-get-conn
branch
from
d4a1e73 to
f261609
Compare
| conn = self.get_connection(self.conn_id) | ||
| return conn.host | ||
| extras = conn.extra_dejson | ||
| openai_client_kwargs = extras.get("openai_client_kwargs", {}) |
There was a problem hiding this comment.
This means they have to do {"openai_client_kwargs":{"arg1":2,"arg2": ["2","6"], "arg3": '{"a":5}'} which I think is a lot. My preference is to have them list out the args without openai_client_kwargs. That's more popular in the hooks here
There was a problem hiding this comment.
I did consider the alternative of listing out the arguments directly. The reason for using the flexible nested dict openai_client_kwargs is mainly driven by the rapidly evolving nature of the OpenAI library. With each update, the client may introduce changes such as renaming existing arguments, adding new ones, or dropping some altogether. By encapsulating the arguments within a nested dict, our hook becomes less prone to immediate disruptions caused by such changes and avoids higher maintenance overhead as it offers adaptability to changes in the OpenAI library without requiring immediate updates on our end.
Since fields under extra already go in a dict and are not direct fields in the UI form, I don't think having another nested key opeai_client_kwargs would be a lot.
It's just that instead of
extra={"arg1":2,"arg2": ["2","6"], "arg3": '{"a":5}'},
it goes one level deep
extra={"openai_client_kwargs":{"arg1":2,"arg2": ["2","6"], "arg3": '{"a":5}'}}
and offers low maintenance.
(cherry picked from commit d2514b4)
ephraimbuddy
added
type:misc/internal
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.