You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "josh-fell (via GitHub)" <gi...@apache.org> on 2023/09/05 14:32:23 UTC
[GitHub] [airflow] josh-fell opened a new pull request, #34106: Use `cached_property` for hook in SparkKubernetesSensor
josh-fell opened a new pull request, #34106:
URL: https://github.com/apache/airflow/pull/34106
Best practice to is make constructors of operators/sensors as simple as possible. Currently this sensor directly builds a KubernetesHook object in its construction. Yes, there are db or external calls when building the hook object itself, but:
1. if this hook changes for any reason, these types calls might sneak in
2. doesn't allow for templating of `kubernetes_conn_id` as brought up in [Airflow Slack](https://apache-airflow.slack.com/archives/CCQ7EGB1P/p1693904864661079)
--
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] jedcunningham merged pull request #34106: Use `cached_property` for hook in SparkKubernetesSensor
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #34106:
URL: https://github.com/apache/airflow/pull/34106
--
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 diff in pull request #34106: Use `cached_property` for hook in SparkKubernetesSensor
Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #34106:
URL: https://github.com/apache/airflow/pull/34106#discussion_r1315995816
##########
tests/providers/cncf/kubernetes/sensors/test_spark_kubernetes.py:
##########
@@ -552,13 +552,6 @@
class TestSparkKubernetesSensor:
def setup_method(self):
db.merge_conn(Connection(conn_id="kubernetes_default", conn_type="kubernetes", extra=json.dumps({})))
- db.merge_conn(
Review Comment:
Completely duplicated line so I removed it.
--
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