davlum commented on a change in pull request #17387:
URL: https://github.com/apache/airflow/pull/17387#discussion_r681851621
##########
File path: airflow/providers/hashicorp/_internal_client/vault_client.py
##########
@@ -191,8 +191,21 @@ def __init__(
self.radius_secret = radius_secret
self.radius_port = radius_port
+ @property
+ def client(self):
+ """
+ Wrapper function to invalidate cache
+
+ :rtype: hvac.Client
+ :return: Vault Client
+
+ """
+ if '_client' in self.__dict__ and not self._client.is_authenticated():
+ del self.__dict__['_client']
+ return self._client
Review comment:
I'm not sure it is easier to understand. The assignment here is
implicitly making sure that `_client` is inside `__dict__`, whereas the `if
'_client' in self.__dict__` is explicit. I agree that modifying `__dict__` is
hacky, but as mentioned this is just how `cached_property` works and is
documented.
I'm not sure that we want to use the `lru_cache`, as it could be
semantically confusing here. There is no benefit to using a lru except for the
convenient `.cache_clear()`. I think adding a comment is definitely a good idea
though.
--
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]
