Skip to content

Conversation

@mingdaoy
Copy link
Contributor

@mingdaoy mingdaoy commented Jul 3, 2025

Related #52676


^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Copy link
Contributor

@dominikhei dominikhei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you importing the BaseHook from facebook.version_compat and not from openai?

@amoghrajesh
Copy link
Contributor

@mingdaoy there is a static check failure:

diff --git a/providers/openai/src/airflow/providers/openai/hooks/openai.py b/providers/openai/src/airflow/providers/openai/hooks/openai.py
index 81c120e..5b0566a 100644
--- a/providers/openai/src/airflow/providers/openai/hooks/openai.py
+++ b/providers/openai/src/airflow/providers/openai/hooks/openai.py
@@ -44,7 +44,6 @@ if TYPE_CHECKING:
     )
     from openai.types.vector_stores import VectorStoreFile, VectorStoreFileBatch, VectorStoreFileDeleted
 from airflow.providers.openai.exceptions import OpenAIBatchJobException, OpenAIBatchTimeout
-
 from airflow.providers.openai.version_compat import BaseHook
 
 

This error means that you have to fix the issues listed above:

The best way to ensure this doesn't happen is to have precommit set up locally: https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#pre-commit-hooks

It will run all these checks for you and save a lot of time!

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should get through now, thanks!

@amoghrajesh
Copy link
Contributor

Ah, the redis tests are unrelated and are tracked here: #52906

@amoghrajesh
Copy link
Contributor

Right, we are green now!

@amoghrajesh amoghrajesh merged commit 15eadf7 into apache:main Jul 6, 2025
71 checks passed
HsiuChuanHsu pushed a commit to HsiuChuanHsu/airflow that referenced this pull request Jul 10, 2025
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
stephen-bracken pushed a commit to stephen-bracken/airflow that referenced this pull request Jul 15, 2025
Co-authored-by: Amogh Desai <amoghrajesh1999@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants