[GitHub] [airflow] tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type
tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type URL: https://github.com/apache/airflow/pull/6665#issuecomment-559303922 yeah.. I agree that.. I wish all user would replace it to full pod spec instead additional args. Thanks to Response on my PR. [Close] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type
tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type URL: https://github.com/apache/airflow/pull/6665#issuecomment-55779 Hi @davlum I thought there are some awkward point in args of KubernetesPodOperator. There are classes we optionally use for KubernetesPodOperator. Port, Volume, VolumeMount, Secret, PodRuntimeInfoEnv, Resources. But only Resource passed by `dict` type. In my exp of 1.10.4, i was confused what args passed should be dict or class type So, there are two way to unify of them. 1. Resource could be changed Resource Type ``` def __init__(self, ..., resources: Optional[Resources] = None ...) ``` But, from using KubernetesPodOperator, we need to import all using class. 2. Others args could be passed by `dict` Type. like my PR. But as you say, if `kubernetes.client.models` is better, we could extract only `attach_to_pod` method previous class based `K8SModel`. (e.g. `PodRuntimeInfoEnv` -> `V1EnvVar`). And just pass them all instance of kubernetes.client.models 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type
tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type URL: https://github.com/apache/airflow/pull/6665#issuecomment-55779 Hi @davlum I thought there are some awkward point in args of KubernetesPodOperator. There are classes we optionally use for KubernetesPodOperator. Port, Volume, VolumeMount, Secret, PodRuntimeInfoEnv, Resources. But only Resource passed by `dict` type. In my exp of 1.10.4, i was confused what args passed should be dict or class type So, there are two way to unify of them. 1. Resource could be changed Resource Type ``` def __init__(self, ..., resources: Optional[Resources] = None ...) ``` But, from using KubernetesPodOperator, we need to import all using class. 2. Others args could be passed by `dict` Type. like my PR. But as you say, if `kubernetes.client.models` is better, we could extract only `attach_to_pod` method previous class based `K8SModel`. (e.g. `PodRuntimeInfoEnv` -> `V1EnvVar`). And just pass them all instance from kubernetes.client.models 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type
tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type URL: https://github.com/apache/airflow/pull/6665#issuecomment-55779 Hi @davlum I thought there are some awkward point in args of KubernetesPodOperator. There are classes we optionally use for KubernetesPodOperator. Port, Volume, VolumeMount, Secret, PodRuntimeInfoEnv, Resources. But only Resource passed by `dict` type. In my exp of 1.10.4, i was confused what args passed should be dict or class type So, there are two way to unify of them. 1. Resource could be changed Resource Type ``` def __init__(self, ..., resources: Optional[Resources] = None ...) ``` But, from using KubernetesPodOperator, we need to import all using class. 2. Others args could be passed by `dict` Type. like my PR. But as you say, if `kubernetes.client.models` is better, we could deprecated previous class based `K8SModel`. (e.g. `PodRuntimeInfoEnv` -> `V1EnvVar`). And just pass them all instance from kubernetes.client.models 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services