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 the deferrable mode to the Dataflow sensors #37693

Conversation

e-galan
Copy link
Contributor

@e-galan e-galan commented Feb 25, 2024

Add the deferrable mode for the Dataflow sensors.

Changes:

  • Deferrable methods for all Dataflow sensors;
  • New trigger classes: DataflowJobAutoScalingEventTrigger, DataflowJobStatusTrigger, DataflowJobMetricsTrigger and DataflowJobMessagesTrigger ;
  • A new example dag;
  • New methods for AsyncDataflowHook;
  • Added unit tests to cover the new code;
  • Fixed some of the existing Dataflow unit tests.

^ 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 Feb 25, 2024
Copy link

boring-cyborg bot commented Feb 25, 2024

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: [email protected]
    Slack: https://s.apache.org/airflow-slack

@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from a3a6ca4 to 77f2ac0 Compare February 26, 2024 12:59
@e-galan e-galan marked this pull request as draft March 15, 2024 09:25
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from 77f2ac0 to b3597f7 Compare March 15, 2024 09:26
@e-galan e-galan marked this pull request as ready for review March 15, 2024 09:32
@VladaZakharova
Copy link
Contributor

Hi @eladkal @Lee-W @hussein-awala !
Can you please the changes? Thanks :)

@e-galan e-galan marked this pull request as draft March 15, 2024 10:22
@Lee-W Lee-W self-requested a review March 15, 2024 16:31
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

not yet finish the whole reviewing. left a few nitpicks

airflow/providers/google/cloud/sensors/dataflow.py Outdated Show resolved Hide resolved
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch 2 times, most recently from ffe164a to a6f89fc Compare March 25, 2024 13:09
@e-galan e-galan changed the title Add the deferrable mode to DataflowJobAutoScalingEventsSensor and DataflowJobMessagesSensor. Add the deferrable mode to the Dataflow sensors Mar 27, 2024
@e-galan e-galan marked this pull request as ready for review March 27, 2024 09:02
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from a6f89fc to 661ff82 Compare March 28, 2024 09:51
@e-galan e-galan requested a review from Lee-W April 3, 2024 09:33
airflow/providers/google/cloud/sensors/dataflow.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/sensors/dataflow.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/triggers/dataflow.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/triggers/dataflow.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/triggers/dataflow.py Outdated Show resolved Hide resolved
@Lee-W Lee-W self-requested a review April 5, 2024 15:59
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from 661ff82 to f3c2714 Compare April 8, 2024 10:06
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from f3c2714 to d2290e6 Compare April 10, 2024 13:36
@e-galan e-galan requested a review from Lee-W April 10, 2024 13:38
@VladaZakharova
Copy link
Contributor

Hi @Lee-W @potiuk !
Can we please merge the changes? Thanks!

@Lee-W Lee-W self-requested a review April 14, 2024 04:35
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me. One last nitpick.

tests/providers/google/cloud/triggers/test_dataflow.py Outdated Show resolved Hide resolved
tests/providers/google/cloud/triggers/test_dataflow.py Outdated Show resolved Hide resolved
tests/providers/google/cloud/triggers/test_dataflow.py Outdated Show resolved Hide resolved
@e-galan e-galan force-pushed the add-deferrable-mode-to-dataflow-job-auto-scaling-events-sensor branch from d2290e6 to 6222d42 Compare April 16, 2024 11:38
@e-galan
Copy link
Contributor Author

e-galan commented Apr 17, 2024

Hi @Lee-W and @potiuk ! Could anyone merge this PR please?

@Lee-W Lee-W merged commit b41cf62 into apache:main Apr 18, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants