Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-21 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1822023611 Apologies, I pushed a very bad rebase to the [previous PR](https://github.com/apache/airflow/pull/35770) which requested review from a dozen contributors. Reopening a separate PR to k

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-22 Thread via GitHub
vincbeck commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1823051016 > Apologies, I pushed a very bad rebase to the [previous PR](https://github.com/apache/airflow/pull/35770) which requested review from a dozen contributors. Reopening a separate PR to

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-22 Thread via GitHub
vincbeck commented on code in PR #35790: URL: https://github.com/apache/airflow/pull/35790#discussion_r1402331833 ## airflow/providers/amazon/aws/operators/ec2.py: ## @@ -254,3 +255,128 @@ def execute(self, context: Context): "MaxAttempts": self.max_atte

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-22 Thread via GitHub
o-nikolas commented on code in PR #35790: URL: https://github.com/apache/airflow/pull/35790#discussion_r1402634064 ## airflow/providers/amazon/aws/operators/ec2.py: ## @@ -254,3 +255,128 @@ def execute(self, context: Context): "MaxAttempts": self.max_att

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-22 Thread via GitHub
dashton90 commented on code in PR #35790: URL: https://github.com/apache/airflow/pull/35790#discussion_r1402836371 ## tests/system/providers/amazon/aws/example_ec2.py: ## @@ -142,6 +145,22 @@ def parse_response(instance_ids: list): ) # [END howto_sensor_ec2_instance_st

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
vincbeck merged PR #35790: URL: https://github.com/apache/airflow/pull/35790 -- 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 unsubscribe, e-mail: commits-unsubscr...@airflow

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825004349 This merge broke the system tests; t4g.micro instances can not be hibernated. Did you test this out? If so, I don't suppose you remember which instance size worked for you? `

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825009160 Looking through the code, it looks like you are checking for a hibernation option, but the exception you are raising `raise AirflowException(f"Instance {instance['InstanceId']} is not

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825021497 I tested on an m4.large instance And didn't come across as harsh at all. Please let me know how I can assist with the system tests. -- This is an automated message from the A

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825022256 As a separate note, are these tests available publicly? Would be happy to follow any additional steps before opening a PR in the future -- This is an automated message from the Apac

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825024215 What do you think about the unit tests? Do they need to be tweaked? > I tested on an m4.large instance Cool. I'll see if I can find documentation around which sizes DO w

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825027155 > Cool. I'll see if I can find documentation around which sizes DO work, but at least we have a place to start now. The supported instances are listed [here](https://docs.aws.a

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825031115 > The supported instances are listed [here](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/hibernating-prerequisites.html#hibernation-prereqs-supported-instance-families) Yo

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825031778 Just re-reading through your comments: > Looking through the code, it looks like you are checking for a hibernation option, but the exception you are raising raise AirflowExcept

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825035261 > If you'd like, I can update the example to use an m4 or t3 instance. I'm running it against my live AWS account. It is going need a few other changes, too. The AMI architect

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825035554 > The example_ec2 DAG would never get to this operator since it is failing during instance creation. Yup, good catch. My mistake. -- This is an automated message from the Apa

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825038027 > Do you want to poke at it and get it sorted out, or do you want me to? If you're already on it then I'll leave it to you. But happy to take over if you need another set of eye

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-23 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825044746 It looks like in order to get this working we're going to need to set up a KMS key and manually define the device mappings to configure an EBS volume. I won't be able to get this done

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-24 Thread via GitHub
dashton90 commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825805238 Created https://github.com/apache/airflow/pull/35839. I've tested the changes independently, but can I leave it to you to do the end-to-end tests? -- This is an automated message fr

Re: [PR] Add EC2HibernateInstanceOperator and EC2RebootInstanceOperator [airflow]

2023-11-24 Thread via GitHub
ferruzzi commented on PR #35790: URL: https://github.com/apache/airflow/pull/35790#issuecomment-1825956441 Amazing, thanks! I'll have a look later today. -- 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