You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/27 19:33:06 UTC
[GitHub] [airflow] josh-fell opened a new pull request #20526: Update connection object to cached_property in DatabricksHook
josh-fell opened a new pull request #20526:
URL: https://github.com/apache/airflow/pull/20526
This PR:
1. Moves the database call out of the hook's `__init__()` method (there has been some back-and-forth on PRs recently for this).
2. Intends to handle any Mypy errors.
Both of these issues should be solved by using a `cached_property` on the `self.databricks_conn` object.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #20526: Update connection object to cached_property in DatabricksHook
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #20526:
URL: https://github.com/apache/airflow/pull/20526
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on a change in pull request #20526: Update connection object to cached_property in DatabricksHook
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #20526:
URL: https://github.com/apache/airflow/pull/20526#discussion_r776029018
##########
File path: airflow/providers/databricks/hooks/databricks.py
##########
@@ -305,6 +311,11 @@ def _do_api_call(self, endpoint_info, json):
"""
method, endpoint = endpoint_info
+ if 'host' in self.databricks_conn.extra_dejson:
Review 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 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #20526: Update connection object to cached_property in DatabricksHook
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20526:
URL: https://github.com/apache/airflow/pull/20526#issuecomment-1001715671
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.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] josh-fell commented on pull request #20526: Update connection object to cached_property in DatabricksHook
Posted by GitBox <gi...@apache.org>.
josh-fell commented on pull request #20526:
URL: https://github.com/apache/airflow/pull/20526#issuecomment-1001713665
CC @dstandish @kaxil
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a change in pull request #20526: Update connection object to cached_property in DatabricksHook
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #20526:
URL: https://github.com/apache/airflow/pull/20526#discussion_r775650475
##########
File path: airflow/providers/databricks/hooks/databricks.py
##########
@@ -305,6 +311,11 @@ def _do_api_call(self, endpoint_info, json):
"""
method, endpoint = endpoint_info
+ if 'host' in self.databricks_conn.extra_dejson:
Review 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
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org