[GitHub] [airflow] ashb commented on a change in pull request #5685: [AIRFLOW-5072] gcs_hook's download() method should download only once

[GitBox <gi...@apache.org>]

ashb commented on a change in pull request #5685: [AIRFLOW-5072] gcs_hook's download() method should download only once
URL: https://github.com/apache/airflow/pull/5685#discussion_r319948552
 
 

 ##########
 File path: airflow/contrib/operators/gcs_download_operator.py
 ##########
 @@ -32,6 +32,10 @@
 class GoogleCloudStorageDownloadOperator(BaseOperator):
     """
     Downloads a file from Google Cloud Storage.
+    When no filename is supplied, the operator loads the file into memory and returns its
 
 Review comment:
   This isn't quite true for an operator as operators don't have a return value, so it's the store_to_xcom_key that controls the behaviour.
   
   As a result of your change in the hook `store_to_xcom_key` and `filename` are now mutually exclusive, which makes this a breaking change to the operator behaviour. 
   
   So either you need to add a note to UPDATING.md about this change (and ensure that either store_to_xcom_key is True and filename is empty) or make the operator load the contents form disk when store_to_xcom_key is True and filename is not empty.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services