Skip to content

[AIRFLOW-6978] Add PubSubPullOperator#7613

Closed
ANiteckiP wants to merge 26 commits into
apache:masterfrom
ANiteckiP:AIRFLOW-6978-pubsub-pull-operator
Closed

[AIRFLOW-6978] Add PubSubPullOperator#7613
ANiteckiP wants to merge 26 commits into
apache:masterfrom
ANiteckiP:AIRFLOW-6978-pubsub-pull-operator

Conversation

@ANiteckiP

@ANiteckiP ANiteckiP commented Mar 3, 2020

Copy link
Copy Markdown
Contributor

Add PubSubPullOperator - a non-blocking equivalent to Add PubSubPullSensor.

These two share most of functionality but ultimately serve different purpose - while existing PubSubPullSensor will block the pipeline until it receives a message, new PubSubPullOperator can be used to check for new messages opportunistically.


Issue link: AIRFLOW-6978

Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
  • Unit tests coverage for changes (not needed for documentation changes)
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

* For document-only changes commit message can start with [AIRFLOW-XXXX].


In case of fundamental code change, 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 UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg Bot added the provider:google Google (including GCP) related issues label Mar 3, 2020
@ANiteckiP ANiteckiP changed the title [GCP Operators] Add PubSubPullOperator [AIRFLOW-6978] Add PubSubPullOperator Mar 3, 2020
@turbaszek turbaszek requested review from mik-laj and turbaszek March 3, 2020 20:43

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice ! Only one question/comment for empty arrays

Comment thread airflow/providers/google/cloud/hooks/pubsub.py

return ret

def _default_message_callback(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

:ref:`howto/operator:PubSubPullSensor`

.. seealso::
If you don't want to wait for at least one message to come, use Operator instead:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good comment

:type max_messages: int
:param return_immediately: If True, instruct the PubSub API to return
immediately if no messages are available for delivery.
:param return_immediately:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here too! I love the detailed explanations!

Comment thread airflow/providers/google/cloud/sensors/pubsub.py
@michalslowikowski00

Copy link
Copy Markdown
Contributor

Will you squash all commits?

@ANiteckiP ANiteckiP changed the title [AIRFLOW-6978] Add PubSubPullOperator WIP: [AIRFLOW-6978] Add PubSubPullOperator Mar 5, 2020
@ANiteckiP

Copy link
Copy Markdown
Contributor Author

WIP: turned out I've missed some docs to be updated!

@michalslowikowski00

Copy link
Copy Markdown
Contributor

Aw... this is WIP. :) I haven't noticed

@turbaszek turbaszek requested a review from potiuk March 7, 2020 10:05
Comment thread airflow/providers/google/cloud/sensors/pubsub.py Outdated
Comment thread tests/providers/google/cloud/hooks/test_pubsub.py Outdated
Comment thread tests/providers/google/cloud/hooks/test_pubsub.py Outdated
Comment thread tests/providers/google/cloud/hooks/test_pubsub.py Outdated

@turbaszek turbaszek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Please adjust PR title once it's ready :)

@ANiteckiP ANiteckiP force-pushed the AIRFLOW-6978-pubsub-pull-operator branch from fb6239f to 5c5a74a Compare March 16, 2020 11:06
@ANiteckiP ANiteckiP changed the title WIP: [AIRFLOW-6978] Add PubSubPullOperator [AIRFLOW-6978] Add PubSubPullOperator Mar 16, 2020
@turbaszek

Copy link
Copy Markdown
Member

Done in #7766

@turbaszek turbaszek closed this Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants