Repository navigation
Use waiters in ECS Operators instead of inner sensors - #29761
Conversation
boring-cyborg
Bot
added
area:providers
provider:amazon
There was a problem hiding this comment.
To be honest this happen almost always 🤣
There was a problem hiding this comment.
This for allow comparison EcsClusterStates.ACTIVE == "ACTIVE" unfortunetly class StrEnum available only in python 3.11
There was a problem hiding this comment.
Yeah, I was hoping to use StrEnum there but... not yet. :( I didn't know you could do it this way, nice.
1a1868b to
7a81b8e
Compare
ferruzzi
left a comment
ferruzzi
left a comment
There was a problem hiding this comment.
Left a couple of suggestions in the waiter config file specifically. Non-blocking.
There was a problem hiding this comment.
Yeah, I was hoping to use StrEnum there but... not yet. :( I didn't know you could do it this way, nice.
There was a problem hiding this comment.
Love seeing this put to use. I think it's so much cleaner in the implementation. 👍
There was a problem hiding this comment.
Hehe, I've looked into builtin botocore waiters for ECS: https://github.com/boto/botocore/blob/develop/botocore/data/ecs/2014-11-13/waiters-2.json
And after that I checked which state available for different API calls https://docs.aws.amazon.com/AmazonECS/latest/developerguide/api_failures_messages.html
There was a problem hiding this comment.
No failure states for this one? I think if the status goes to 'DELETE_IN_PROGRESS' it can fail early. Not sure where 'INACTIVE' is on the state flow on this call though.
There was a problem hiding this comment.
Yeah 'DELETE_IN_PROGRESS' it could be some kind failure step, I can't imagine how it possible that state changed to this state during create task definition, but why not to add it as failure.
There was a problem hiding this comment.
Pretty much my thought as well. I don't how it would get there, but if it did then something is clearly wrong so there's no point waiting for the timeout.
There was a problem hiding this comment.
Would 'DELETE_IN_PROGRESS' be a success state here?
There was a problem hiding this comment.
I'd rather say it intermediate state in case of delete task definition.
There was a problem hiding this comment.
Nice. I have a bunch of EMR waiters I have been workigb on converting over but put on pause to verify how they behave if None is passed in, but this would solve that nncely. I'll get the EMR ones up this week and tag you in them. 👍
7a81b8e to
d3928d7
Compare
o-nikolas
left a comment
o-nikolas
left a comment
There was a problem hiding this comment.
Nice addition!
Just some wording suggestions and a question or two.
Co-authored-by: Niko Oliveira <onikolas@amazon.com>
ferruzzi
left a comment
ferruzzi
left a comment
There was a problem hiding this comment.
Looks like everything got fixed. Approved 👍
|
ECS System test is failing after this merge, the |
And this a bit strange, because after this logger record, as I could see there is no external calls (to AWS) and only simple validation which should finished in a milliseconds |
|
Yep locally it also checked forever I guess it is combination of airflow serializer, templated fields and multi inheritance (which I add for enums)
|
closes: #29556
Use custom botocore.Waiters instead of inner sensors in ECS Operators