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]

Reply via email to