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 2020/08/21 13:14:03 UTC

[GitHub] [airflow] turbaszek opened a new pull request #10446: Add kubernetes connection type

turbaszek opened a new pull request #10446:
URL: https://github.com/apache/airflow/pull/10446


   Add kubernetes connection type 'required' for Spark on k8s
   operator from backport packages.
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/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/master/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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-687004245


   @mik-laj this change was is for 1.10.X so I don't know if we even want to merge it. Once 2.0 is out this will be redundant 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil edited a comment on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678311136


   > @kaxil but, don't we have user input validation on conn type in WebUI?
   > <img alt="Screenshot 2020-08-21 at 15 40 04" width="740" src="https://user-images.githubusercontent.com/9528307/90896826-991aff80-e3c4-11ea-9990-2afb6bfb7669.png">
   
   You can create a connection with any Connection type, example:
   
   ```
   ❯ airflow connections -a --conn_id "a_test" --conn_type "my_custom_type" --conn_host xyz
   
   	Successfully added `conn_id`=a_test : my_custom_type://:******@xyz:
   ```
   
   Airflow doesn't validate that connection type (<=1.10.12) is what I meant :)
   
   ![image](https://user-images.githubusercontent.com/8811558/90899904-89012100-e3c0-11ea-9ec4-e256d284b2ba.png)
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-686817157


   @turbaszek Be warned that this PR does not run all tests on CI.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil closed pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
kaxil closed pull request #10446:
URL: https://github.com/apache/airflow/pull/10446


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678435531


   My questions was answered beautifully. Thanks @kaxil @turbaszek !


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] potiuk edited a comment on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678435531


   My question was answered beautifully. Thanks @kaxil @turbaszek !


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678291097


   Does it mean that it won't work for 1.10.12 :( @turbaszek ? Or should we make some workaround ?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678300185


   And we do enforce:
   ```
   The following args are required to add a connection: ['conn_uri or conn_type']
   ```


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678296209


   @kaxil but, don't we have user input validation on conn type in WebUI?
   <img width="740" alt="Screenshot 2020-08-21 at 15 40 04" src="https://user-images.githubusercontent.com/9528307/90896826-991aff80-e3c4-11ea-9990-2afb6bfb7669.png">
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] turbaszek edited a comment on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678327477


   Using cli - right. Using webui - no. You can't create "custom conn type" due to input validation. No strong opinion here, I just thought that having this conn type may help users who like me wanted to use web ui 😄 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678311136


   > @kaxil but, don't we have user input validation on conn type in WebUI?
   > <img alt="Screenshot 2020-08-21 at 15 40 04" width="740" src="https://user-images.githubusercontent.com/9528307/90896826-991aff80-e3c4-11ea-9990-2afb6bfb7669.png">
   
   You can create a connection with any Connection type, example:
   
   ```
   ❯ airflow connections -a --conn_id "a_test" --conn_type "my_custom_type" --conn_host xyz
   
   	Successfully added `conn_id`=a_test : my_custom_type://:******@xyz:
   ```
   
   Airflow doesn't validate that connection ID is what I meant :)
   
   ![image](https://user-images.githubusercontent.com/8811558/90899904-89012100-e3c0-11ea-9ec4-e256d284b2ba.png)
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678292146


   Well... the main reason for this change is that I wanted to try to add such connection using 1.10.12 WebUI. But we have conn_type validation and there's no option to add a custom one. Of course, I can use any other type and just add proper extras but that sounds like asking for troubles. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] turbaszek commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678327477


   Using cli - right. Using webui - no. You can't create "custom conn type". 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678335857


   > Using cli - right. Using webui - no. You can't create "custom conn type" due to input validation. No strong opinion here, I just thought that having this conn type may help users who like me wanted to use web ui 😄
   
   Oh yeah don't get me wrong -- This PR is completely needed -- the stuff I said is just a workaround (like UI you could use any other connection type). I was just replying to Jarek comment "Does it mean that it won't work for 1.10.12 :( @turbaszek ? Or should we make some workaround?" :)


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #10446: Add kubernetes connection type

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #10446:
URL: https://github.com/apache/airflow/pull/10446#issuecomment-678293237


   No it should work for all Airflow versions < 1.10.12 as Connection Type is not enforced in those versions


----------------------------------------------------------------
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:
users@infra.apache.org