Skip to content

add deferrable mode to RedshiftDataOperator - #36586

Merged
eladkal merged 11 commits into
apache:mainfrom
astronomer:add-deferrable-mode-to-RedshiftDataOperator
Jan 18, 2024
Merged

add deferrable mode to RedshiftDataOperator#36586
eladkal merged 11 commits into
apache:mainfrom
astronomer:add-deferrable-mode-to-RedshiftDataOperator

Conversation

@Lee-W

@Lee-WLee-W commented Jan 4, 2024

Copy link
Copy Markdown
Member

^ 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-cyborgboring-cyborgBot added area:providers provider:amazon AWS/Amazon - related issues labels Jan 4, 2024
@Lee-WLee-W mentioned this pull request Jan 4, 2024
Closed
19 tasks
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch 5 times, most recently from eb6277c to d4ece3dCompareJanuary 5, 2024 03:45
dirrao
dirrao requested changes Jan 6, 2024
View reviewed changes

@dirraodirrao left a comment

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.

Nice work, I have given one comment. It would be great if we can add the test cases.

Comment threadairflow/providers/amazon/aws/operators/redshift_data.py Outdated
dirrao
dirrao requested changes Jan 9, 2024
View reviewed changes

@dirraodirrao left a comment

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.

Couple of checks are failing. Can you add the units for the deferrable mode?

@Lee-WLee-W mentioned this pull request Jan 9, 2024
Merged
@Lee-W

Lee-W commented Jan 13, 2024

Copy link
Copy Markdown
MemberAuthor

Couple of checks are failing. Can you add the units for the deferrable mode?

Yep, this is still a work in progress. Will try to wrap it up next week

@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch 2 times, most recently from a8aa91d to 86e43e9CompareJanuary 15, 2024 14:40
@Lee-W
Lee-W marked this pull request as ready for review January 15, 2024 14:50
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch 2 times, most recently from b5cb9cd to 8f59d6fCompareJanuary 16, 2024 02:22
Taragolis
Taragolis reviewed Jan 16, 2024
View reviewed changes
Comment threadairflow/providers/amazon/aws/triggers/redshift_data.py Outdated
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch from 6a7b3c5 to c14e36aCompareJanuary 16, 2024 06:49
syedahsn
syedahsn reviewed Jan 16, 2024
View reviewed changes
Comment threadairflow/providers/amazon/aws/hooks/redshift_data.py Outdated
syedahsn
syedahsn reviewed Jan 16, 2024
View reviewed changes
Comment threadairflow/providers/amazon/aws/hooks/redshift_data.py Outdated
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch 2 times, most recently from 18e157e to 5fe6958CompareJanuary 17, 2024 10:44
Taragolis
Taragolis approved these changes Jan 17, 2024
View reviewed changes

@TaragolisTaragolis left a comment

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.

Looks good to me, just one small nit from my side about property/cached_property

Comment threadairflow/providers/amazon/aws/triggers/redshift_data.py Outdated
syedahsn
syedahsn reviewed Jan 17, 2024
View reviewed changes
Comment threadairflow/providers/amazon/aws/hooks/redshift_data.py Outdated
syedahsn
syedahsn reviewed Jan 17, 2024
View reviewed changes
Comment threadairflow/providers/amazon/aws/hooks/redshift_data.py Outdated
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch from 5fe6958 to 3eea497CompareJanuary 18, 2024 04:37
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch from 6d94f80 to e1e9b4fCompareJanuary 18, 2024 08:14
@Lee-W
Lee-Wforce-pushed the add-deferrable-mode-to-RedshiftDataOperator branch from e1e9b4f to d0a7d1cCompareJanuary 18, 2024 08:41
dirrao
dirrao approved these changes Jan 18, 2024
View reviewed changes

@dirraodirrao left a comment

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.

LGTM

@Lee-W
Lee-W requested a review from syedahsnJanuary 18, 2024 13:35
syedahsn
syedahsn approved these changes Jan 18, 2024
View reviewed changes

@syedahsnsyedahsn left a comment

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.

Nice work!

@eladkal
eladkal merged commit 9563dc5 into apache:mainJan 18, 2024
@eladkaleladkal mentioned this pull request Jan 22, 2024
Closed
@borismoborismo mentioned this pull request Aug 7, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TaragolisTaragolisTaragolis approved these changes
@dirraodirraodirrao approved these changes
@eladkaleladkaleladkal approved these changes
@o-nikolaso-nikolaso-nikolas approved these changes
+1 more reviewer
@syedahsnsyedahsnsyedahsn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providersprovider:amazonAWS/Amazon - related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Lee-W@Taragolis@dirrao@eladkal@o-nikolas@syedahsn