Skip to content

Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode - #34216

Merged
potiuk merged 5 commits into
apache:mainfrom
yermalov-here:aws_EmrAddStepsOperator_deferred
Sep 11, 2023
Merged

Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode#34216
potiuk merged 5 commits into
apache:mainfrom
yermalov-here:aws_EmrAddStepsOperator_deferred

Conversation

@yermalov-here

@yermalov-here yermalov-here commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

This PR reimplements EmrAddStepsTrigger based on the AwsBaseWaiterTrigger to be used in the EmrAddStepsOperator in deferred mode.

related: #34183
closes: #34177


^ 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.

* to be used by the EmrAddStepsOperator in deferred mode
@boring-cyborg boring-cyborg Bot added area:providers provider:amazon AWS/Amazon - related issues labels Sep 8, 2023
@yermalov-here yermalov-here changed the title WIP: Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode Sep 8, 2023
@yermalov-here yermalov-here changed the title Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode WIP: Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode Sep 8, 2023
@yermalov-here
yermalov-here marked this pull request as draft September 8, 2023 15:10
@yermalov-here
yermalov-here marked this pull request as ready for review September 8, 2023 15:48
@yermalov-here yermalov-here changed the title WIP: Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode Use a AwsBaseWaiterTrigger-based trigger in EmrAddStepsOperator deferred mode Sep 8, 2023
@yermalov-here

Copy link
Copy Markdown
Contributor Author

Please note that i'd be glad to update this PR, especially based on the answers to the questions mentioned in the description.

@vincbeck

vincbeck commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

@syedahsn @vandonr-amz

@vincbeck

vincbeck commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

Why creating a new trigger and not modifying EmrAddStepsTrigger?

from airflow.providers.amazon.aws.hooks.base_aws import AwsGenericHook


@deprecated(reason="use EmrStepsTrigger instead")

@Taragolis Taragolis Sep 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should deprecate it especially if it never work properly. Just need to fix it so you could move your implementation here

@yermalov-here

yermalov-here commented Sep 9, 2023

Copy link
Copy Markdown
Contributor Author

Replaced the implementation of the EmrAddStepsTrigger
Added tests for the new waiter

@potiuk
potiuk merged commit f0467c9 into apache:main Sep 11, 2023
@yermalov-here
yermalov-here deleted the aws_EmrAddStepsOperator_deferred branch September 12, 2023 06:40
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.

EmrAddStepsOperator does not work with deferrable=True

4 participants