Skip to content

Conversation

@laksh-krishna-sharma
Copy link
Contributor

@laksh-krishna-sharma laksh-krishna-sharma commented Sep 26, 2025

Summary of changes

Closes #56153

  • Deprecated wait_policy parameter in EmrCreateJobFlowOperator.
  • Added wait_for_completion (boolean) as the standard parameter (consistent with other Amazon operators).
  • Preserved backward compatibility by mapping wait_policywait_for_completion.

Changes made

  • Refactored constructor logic to prefer wait_for_completion.
  • Deprecated wait_policy with warning.

Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

Could you please add unit tests to ensure that the deprecation works properly?

@laksh-krishna-sharma
Copy link
Contributor Author

sure @shahar1

@shahar1 shahar1 requested a review from Copilot September 27, 2025 19:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR deprecates the wait_policy parameter in EmrCreateJobFlowOperator and replaces it with wait_for_completion to maintain consistency with other Amazon operators while preserving backward compatibility.

  • Deprecated wait_policy parameter with a warning message
  • Added wait_for_completion boolean parameter as the new standard
  • Updated implementation to use wait_for_completion internally with backward compatibility mapping

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
providers/amazon/src/airflow/providers/amazon/aws/operators/emr.py Updates operator constructor and execute method to use wait_for_completion instead of wait_policy
providers/amazon/tests/unit/amazon/aws/operators/test_emr_create_job_flow.py Updates test cases to use new parameter and adds deprecation warning test

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@shahar1
Copy link
Contributor

shahar1 commented Sep 27, 2025

@laksh-krishna-sharma could you please review Copilot's comments and check if they make sense?

@laksh-krishna-sharma
Copy link
Contributor Author

yes @shahar1 i am reviewing the comments

@laksh-krishna-sharma
Copy link
Contributor Author

@shahar1 the copilot comments make sense. I’m working on resolving these.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@laksh-krishna-sharma
Copy link
Contributor Author

laksh-krishna-sharma commented Sep 27, 2025

@shahar1 Can I mark the above Copilot comments as resolved?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@shahar1 shahar1 self-requested a review September 27, 2025 21:41
@shahar1
Copy link
Contributor

shahar1 commented Sep 27, 2025

@laksh-krishna-sharma please tag me when you're done

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@laksh-krishna-sharma
Copy link
Contributor Author

@shahar1 please review the changes and let me know if i have to refactor

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@laksh-krishna-sharma
Copy link
Contributor Author

@shahar1 please review the changes and let me know if i have to refactor

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.

One nit but overall looks good!

@laksh-krishna-sharma
Copy link
Contributor Author

@vincbeck @shahar1 please review the changes and let me know if i have to refactor

@vincbeck vincbeck merged commit 171c3f3 into apache:main Oct 1, 2025
80 checks passed
@laksh-krishna-sharma laksh-krishna-sharma deleted the refactor/emr-operator-wait-for-completion branch October 1, 2025 13:19
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 2, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 3, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 4, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 7, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 8, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 9, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 10, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 11, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 12, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
dabla pushed a commit to dabla/airflow that referenced this pull request Oct 12, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 14, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
…of wait_for_completion (apache#56158)

* refactor: deprecate wait_policy in EmrCreateJobFlowOperator in favor of wait_for_completion

* added unit test for refactor

* resolved copilot comments

* resolved copilot comments

* fixed failing test

* fixed: refactor of wait_policy

* ensured backward compatibility

* removed "self.wait_policy = wait_policy"
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.

Rename parameter wait_policy to wait_for_completion in EmrCreateJobFlowOperator

3 participants