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
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
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
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
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
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
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?
`
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
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
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
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
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
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
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
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
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
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
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
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
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
20 matches
Mail list logo