Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

custom waiters with dynamic values, applied to appflow #29911

Merged
merged 8 commits into from
Mar 21, 2023

Conversation

vandonr-amz
Copy link
Contributor

@vandonr-amz vandonr-amz commented Mar 3, 2023

  • add a mechanism to allow custom waiters to take dynamic values at runtime by editing the waiter config on the fly

    • placeholders for dynamic values are formated for jinja (between {{}})
    • they are meant to be used in the argument field, which describes what's checked, this is the only place where we replace them
    • the waiter config still needs to be valid json with those placeholders unexpanded
    • there is some validation, unexpanded parameters will send an error, and jinja will complain if the template is not correctly formated.
  • apply this new capability in transforming appflow wait_for_completion code to use a waiter (follow-up on rewrite polling code for appflow hook #28869)

    • not using the execution ID in this waiter would mean making unchecked assumptions on the data we're reading.

airflow/providers/amazon/aws/hooks/base_aws.py Outdated Show resolved Hide resolved
airflow/providers/amazon/aws/hooks/base_aws.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean implementation 👍

"expected": "blah",
"matcher": "path",
"state": "success",
"argument": "{{not a valid jinja template 💀}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOLOLOL 💀 ❤️

f"Parameter was not supplied for templated waiter's acceptor '{arg}'", e
)
return config

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I think I like this solution. I could nitpick the variable names but I always do that. :P

@o-nikolas
Copy link
Contributor

@vincbeck and @Taragolis are you happy with the changes in response to your comments?

@potiuk potiuk merged commit 05c0841 into apache:main Mar 21, 2023
@vandonr-amz vandonr-amz deleted the vandonr/waiter branch May 24, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants