-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a cache for weaviate client #35983
Conversation
AirflowProviderDeprecationWarning, | ||
stacklevel=2, | ||
) | ||
"""Returns a Weaviate client.""" | ||
return self.get_conn() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of removing the deprecation warning from this method, you can make get_conn
a cached property and change this method to:
return self.get_conn() | |
return self.get_conn |
In that way, you will achieve what you want to do without replacing self.get_client()
with self.get_client
, and this method will stay deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think having the cache on get_conn
is a good one but I agree to keep the deprecation. It would be better to have another method like _weaviate_client
or conn
like @Lee-W suggested. It means more options for users and doing hook.get_conn
would be just a statement that IDEs may not fancy.
While working on another issue, I realized how often I had to call get_conn. So instead of depreccating this, we can use it as a cache within the code so we don't connect everytime a method is called.
ca854e3
to
65c2798
Compare
65c2798
to
14b8a2d
Compare
While working on another issue, I realized how often I had to call get_conn. So instead of depreccating this, we can use it as a cache within the code so we don't connect everytime a method is called.