Fix openai 1.32 breaking openai tests #40110
Conversation
|
@potiuk Thanks! They are issuing version updates to their library on an almost daily basis currently. |
b2d148a
to
d079002
Compare
So if it is going to happen more frequently, maybe we will have to change our mocking strategy to be more flexible ? |
Possibly. Or be stricter with the version specification? |
No. We have the rule that we do not keep upper-binding for dependencies unless we know future versions will cause us problems (sqlalchemy, flask etc.) and when we know that that the dependency follows some rule for breaking changes (mostly SemVer). In this case - we do not know if the future change will be breaking - in fact it does not break "user" code only our tests are broken, so limiting our users from upgrading to newer releases is wrong (for example this will force us to release a new version of provider when there is a security fix released by openai. Generally speaking we are well protected by our constraints so upper-binding is not needed. In this case if could have an approach that the mocks are always right (not sure if possible that would be best - maybe somewhere in openai library they have a way to create an "empty" Run object? |
|
Understood. I'll see what I can do. |
4269fc0
to
c2b95e0
Compare
|
It turns out we need to add a bit more limits to google and facebook provider to make sure the lower-direct limits work (otherwise it attempts to build old versions of libraries that fail). |
4cb232a
to
7ea753d
Compare
|
Hah it turned out that it is likely a bug introduced in uv==0.2.6 and for now we should limit uv to 0.2.5 See astral-sh/uv#4136 for details |
The new openai release adds new required parameter `parallel_tool_calls` and our mock did not have it. Bumping version and adding the parameter should solve the problem. It turned out also that uv==0.2.6 introduced a bug for lowest-direct resolution, so we need to limit it to 0.2.5. Details in astral-sh/uv#4136
7ea753d
to
fc19cca
Compare
|
I was too eager to upgrade UV this morning 🤦 .... The bug in uv crossed with attempts to fix the dependencies (@hussein-awala - this is why earlier I warned to be a bit cautious when upgrading to newer UV versions - but apparently I was caught up off-guard as I did it myself :( |
|
Yep. Pinning UV to 0.2.5 fixes the problem. |
The new openai release adds new required parameter `parallel_tool_calls` and our mock did not have it. Bumping version and adding the parameter should solve the problem. It turned out also that uv==0.2.6 introduced a bug for lowest-direct resolution, so we need to limit it to 0.2.5. Details in astral-sh/uv#4136 (cherry picked from commit 65dbf86)
The new openai release adds new required parameter `parallel_tool_calls` and our mock did not have it. Bumping version and adding the parameter should solve the problem. It turned out also that uv==0.2.6 introduced a bug for lowest-direct resolution, so we need to limit it to 0.2.5. Details in astral-sh/uv#4136 (cherry picked from commit 65dbf86)
The new openai release adds new required parameter `parallel_tool_calls` and our mock did not have it. Bumping version and adding the parameter should solve the problem. It turned out also that uv==0.2.6 introduced a bug for lowest-direct resolution, so we need to limit it to 0.2.5. Details in astral-sh/uv#4136
The new openai release adds new required parameter
parallel_tool_callsand our mock did not have it. Bumping version and adding the parameter should solve the problem.^ 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.