MaksYermak commented on code in PR #67621:
URL: https://github.com/apache/airflow/pull/67621#discussion_r3711787722
##########
providers/google/src/airflow/providers/google/cloud/operators/pubsub.py:
##########
Review Comment:
@michaelpri10 did you run the system tests for these changes with
`return_immediately=False`?
I am not sure that we should make this parameter configurable for
`PubSubPullOperator` as I understand the idea of this operator was return any
value immediately include 0 if user does not have any messages. I think it was
a reason why we have hardcoded `return_immediately=True` here. Otherwise with
`return_immediately=False` it blocks whole task until message will not appear
in my opinion this behavior contradicts with operator logic. If users need
blocking waiting for this reason we have `PubSubPullSensor`.
It is the message from docstring to this operator:
```
Pulls messages from a PubSub subscription and passes them through XCom.
If the queue is empty, returns empty list - never waits for messages.
If you do need to wait, please use
:class:`airflow.providers.google.cloud.sensors.PubSubPullSensor`
instead.
```
##########
providers/google/src/airflow/providers/google/cloud/sensors/pubsub.py:
##########
@@ -127,13 +129,21 @@ def __init__(
self.project_id = project_id
self.subscription = subscription
self.max_messages = max_messages
- self.return_immediately = return_immediately
self.ack_messages = ack_messages
self.messages_callback = messages_callback
self.impersonation_chain = impersonation_chain
self.deferrable = deferrable
self.poke_interval = poke_interval
self._return_value = None
+ if return_immediately is not None:
+ warnings.warn(
+ "The default value of `return_immediately` will be changed to
`False` in a future major release.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
Review Comment:
@michaelpri10 in google provider we have deprecation policy and by this
policy you need to specify planned_removal_date and by default all deprecations
should allow a 6 months time period until they will be removed and not
available. Could you please add the removal date to this warning message?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]