Skip to content

Add deferrable mode to dataflow operators #27776

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 30, 2023
Merged

Add deferrable mode to dataflow operators #27776

merged 3 commits into from
Jan 30, 2023

Conversation

MrGeorgeOwl
Copy link
Contributor

@MrGeorgeOwl MrGeorgeOwl commented Nov 18, 2022 •
edited
Loading

^ 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:google Google (including GCP) related issues labels Nov 18, 2022
@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch 5 times, most recently from e12f828 to 7595951 Compare November 24, 2022 11:35
@potiuk
Copy link
Member

potiuk commented Dec 3, 2022

Tests failing.

@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch 3 times, most recently from 079ac53 to 067b705 Compare December 9, 2022 08:42
@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch 5 times, most recently from 7b31c11 to 901352d Compare December 19, 2022 08:23
@MrGeorgeOwl
Copy link
Contributor Author

MrGeorgeOwl commented Dec 19, 2022 •
edited
Loading

@potiuk, tests that are failing at the moment are those that come with changes from main branch. Have the same issues in several PRs so I think that changes in PR are okay but I might be wrong

@potiuk
Copy link
Member

potiuk commented Dec 19, 2022

I think there were intermittent/flaky tests - I re-run them to be sure.

kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
kaxil
kaxil reviewed Dec 21, 2022
View reviewed changes
@brucearctor
Copy link

@MrGeorgeOwl -- this is great and we are eagerly awaiting this functionality to save us some headaches! Do you want some help getting it over the finish line? Looks like the outstanding reviews are pretty straightforward :-)

Please advise.

@MrGeorgeOwl
Copy link
Contributor Author

@brucearctor, thank you for your offer of help I appreciate that. I finished required changes at the moment so PR should be merged after it will be reviewed and approved

@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch 15 times, most recently from 9977d08 to 683f62f Compare January 30, 2023 08:03
uranusjr
uranusjr reviewed Jan 30, 2023
View reviewed changes
uranusjr
uranusjr reviewed Jan 30, 2023
View reviewed changes
potiuk
potiuk requested changes Jan 30, 2023
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Comment why we are adding upper-bound limit

@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch 2 times, most recently from 7fe1466 to 1430b33 Compare January 30, 2023 13:44
@MrGeorgeOwl
Copy link
Contributor Author

@potiuk, I've added comment about upper-bound for google library

@MrGeorgeOwl MrGeorgeOwl force-pushed the dataflow_defferable_mode branch from 1430b33 to e9ede1f Compare January 30, 2023 13:48
@potiuk potiuk merged commit 094d6bf into apache:main Jan 30, 2023
@eladkal eladkal mentioned this pull request Feb 8, 2023
Closed
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uranusjr uranusjr uranusjr left review comments

@bhirsz bhirsz bhirsz left review comments

@kaxil kaxil kaxil left review comments

@potiuk potiuk potiuk approved these changes

@turbaszek turbaszek Awaiting requested review from turbaszek

@ashb ashb Awaiting requested review from ashb

@jedcunningham jedcunningham Awaiting requested review from jedcunningham

@eladkal eladkal Awaiting requested review from eladkal

@o-nikolas o-nikolas Awaiting requested review from o-nikolas

@jhtimmins jhtimmins Awaiting requested review from jhtimmins

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton

@bbovenzi bbovenzi Awaiting requested review from bbovenzi

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy

@dstandish dstandish Awaiting requested review from dstandish

@XD-DENG XD-DENG Awaiting requested review from XD-DENG

Assignees
No one assigned
Labels
area:providers provider:google Google (including GCP) related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MrGeorgeOwl @potiuk @brucearctor @uranusjr @bhirsz @kaxil