allow changing of config file from default for the KubernetesResourceBaseOperator#34819
allow changing of config file from default for the KubernetesResourceBaseOperator#34819chapm250 wants to merge 4 commits into
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
|
The failed tests look unrelated to this change, let me check. |
|
I close an reopen it to execute all the tests |
| @cached_property | ||
| def hook(self) -> KubernetesHook: | ||
| hook = KubernetesHook(conn_id=self.kubernetes_conn_id) | ||
| hook = KubernetesHook(conn_id=self.kubernetes_conn_id, config_file=self.config_file) |
There was a problem hiding this comment.
Dont you think we should have a None check before using the config_file here? If not passed, it will go as None to the hook.
Also can you add some test coverage for this?
There was a problem hiding this comment.
if it's null the kubernetes library fills in the default, the KubernetesPodOperator works the same way
https://github.com/apache/airflow/blob/main/airflow/providers/cncf/kubernetes/operators/pod.py#L301
https://github.com/apache/airflow/blob/main/airflow/providers/cncf/kubernetes/operators/pod.py#L518
There was a problem hiding this comment.
I updated the existing tests for the modified operators in the last commit
|
tests are failing |
amoghrajesh
left a comment
There was a problem hiding this comment.
@chapm250 changes look good, you need to rebase and find out why tests are failing
Currently the KubernetesResourceBaseOperator doesn't allow changing from the default K8s config_file location (unlike the KubernetesPodOperator), this change allows changing the config_file location so airflow deployments with a non-default K8s config_file path can use the subclasses of the KubernetesResourceBaseOperator