[GitHub] [airflow] tyg03485 edited a comment on issue #6665: [AIRFLOW-6068] Make args of KubernetesPodOperator to get dict type

2019-11-27 Thread GitBox
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

2019-11-26 Thread GitBox
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

2019-11-26 Thread GitBox
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

2019-11-26 Thread GitBox
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