[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-08-18 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-901268924 Very strange. It looks fine locally, but you are right, it appears to be empty here on the PR. Just overwrote it with a new file, let's see if that fixes it. It is a 300x3

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-08-10 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-896187912 Hey folks, sorry for the delay. I wanted to get a better understanding of Jinja before I implemented it. I should have another round of updates for you today or tomorrow.

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-31 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-890424892 @andormarkus Hi Andor, can you please submit that as a new Issue? This PR is pretty far along to be changing direction, but I don't see why we couldn't add that for nodegrou

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-28 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-888588265 Pull/Rebased from `main` and added the commit to remove the list and describe operators. -- This is an automated message from the Apache Git Service. To respond to the mess

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-20 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-883716102 Pushed the system tests. They will all skip because the logic for passing/parsing the credentials is not implemented in `provide_aws_context()`, but that feels like it would

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-20 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-882797437 Will do. I have another round of fixes coming today, I'll rebase before I do that. -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-20 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-882797437 Will do. I have another round of fixes coming today, I'll rebase before I do that. -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-19 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-882797437 Will do. I have another round of fixes coming today, I'll rebase before I do that. -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-14 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-880135534 After a discussion with mik-laj on Slack, we agreed to move forward with the 1-hour limit for now, clearly called out in the docs, and start work immediately on a fix for ext

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-13 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-879372031 I see. Thanks. It is definitely a bigger nut to crack than I initially expected. There is always so much to learn. :) -- This is an automated message from the Apac

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-13 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-879328885 Honest question: How common is it for a DAG to run for over an hour? In my limited experience, the DAGs I run generally finish way faster than that, but I'm obviously not d

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-12 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-878659314 I'm not against doing this in multiple PRs. My primary concern is that without the EKSPodOperator, the other Operators and Hooks are not all that useful. I'm not a huge fan

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-07-09 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-877533314 I still have to sort out the docs changes that @mik-laj asked for, but I think this batch of commits should address most/all of the other issues. I misunderstood the co

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-29 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-870978582 Oh what have I done. I did a `git pull --rebase` to get up to date before pushing the changes and it included all of them in this PR...n -- This is an automated message f

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-29 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-869831727 -- 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 unsub

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-28 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-870097190 > > In what way? Are you talking specifically about getting rid of the AWS CLI tool as you mentioned above, or did you have something else in mind? If that is what you are th

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-28 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-870067649 > I think we should think a little more about authorization to a cluster in EKSPodOperator operator. In what way? Are you talking specifically about getting rid of the

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-28 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-869831727 I can add system tests for sure; let's get the rest of the concerns out of the way first. -- This is an automated message from the Apache Git Service. To respond to the mes

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-26 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-869088796 @dimberman @mik-laj @vikramkoka - All CI tests are currently passing, ready for a proper review at your convenience. -- This is an automated message from the Apache Git

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-22 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-866095317 Hi Ash, thanks for the quick review. I'll hit your comments inline, but as far as this one: > Can you explain why you had to change the label-when-reviewed action in

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-22 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-865390141 Not done looking ta all the test results yet, but looks like most/all were caused by either a single misnamed param that slipped through in an example dag, or whitespace issu

[GitHub] [airflow] ferruzzi commented on pull request #16571: Implemented Basic EKS Integration

2021-06-21 Thread GitBox
ferruzzi commented on pull request #16571: URL: https://github.com/apache/airflow/pull/16571#issuecomment-865390141 Not done looking ta all the test results yet, but looks like most/all were caused by either a single misnamed param that slipped through in an example dag, or whitespace issu