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

Add custom waiters to EMR Serverless #30463

Merged

Conversation

syedahsn
Copy link
Contributor

@syedahsn syedahsn commented Apr 4, 2023

This PR replaces the manual waiter with the custom boto waiters. It also adds extra features to the EmrServerlessStartJobOperator:

  • Prints a log message when the job is running.
  • Prints the reason for a job failure if a job fails
    By removing the waiters, the unit tests needed to be updated, and in some cases modified.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Apr 4, 2023
@syedahsn syedahsn force-pushed the syedahsn/emr-serverless-custom-waiters branch 2 times, most recently from 57fae95 to af2a833 Compare April 12, 2023 22:50
Copy link
Contributor

@vandonr-amz vandonr-amz left a comment

Choose a reason for hiding this comment

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

let's go
btw, this is going to conflict with #30720 but happy to let you go first

@potiuk
Copy link
Member

potiuk commented Apr 22, 2023

let's go
btw, this is going to conflict with #30720 but happy to let you go first

As soon as the docs are building with no errors :)

@syedahsn syedahsn force-pushed the syedahsn/emr-serverless-custom-waiters branch from 944d724 to c0e3180 Compare May 31, 2023 12:59
@syedahsn syedahsn force-pushed the syedahsn/emr-serverless-custom-waiters branch from c0e3180 to 97ddbac Compare June 13, 2023 20:50
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

Approving even though I still think using tenacity would be "better", it is just a nit

@syedahsn syedahsn force-pushed the syedahsn/emr-serverless-custom-waiters branch from 97ddbac to 3f3bd4f Compare June 20, 2023 00:47
Copy link
Contributor

@vandonr-amz vandonr-amz left a comment

Choose a reason for hiding this comment

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

LGTM

@o-nikolas o-nikolas merged commit 743bf5a into apache:main Jun 20, 2023
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jun 27, 2023
* Move waiter logic to utils folder

---------

Co-authored-by: Raphaël Vandon <[email protected]>
@vandonr-amz vandonr-amz deleted the syedahsn/emr-serverless-custom-waiters branch September 1, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants