Update connection object to cached_property in DatabricksHook #20526
Conversation
|
CC @dstandish @kaxil |
|
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
| @@ -305,6 +311,11 @@ def _do_api_call(self, endpoint_info, json): | |||
| """ | |||
| method, endpoint = endpoint_info | |||
|
|
|||
| if 'host' in self.databricks_conn.extra_dejson: | |||
There was a problem hiding this comment.
self.host might want to be a cached prop also. or not be an attribute at all. it feels a little uncomfortable to mutate attributes like this, i.e. as side effects of a method
There was a problem hiding this comment.
Ah you're right. Copy pasta. This doesn't need to be an instance attribute. self.host is only referenced in _do_api_call().
This PR:
__init__()method (there has been some back-and-forth on PRs recently for this).Both of these issues should be solved by using a
cached_propertyon theself.databricks_connobject.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.