You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/01/18 12:19:39 UTC

[GitHub] [airflow] RosterIn commented on a change in pull request #12033: AllowDiskUse parameter and docs in MongotoS3Operator

RosterIn commented on a change in pull request #12033:
URL: https://github.com/apache/airflow/pull/12033#discussion_r559525360



##########
File path: tests/providers/amazon/aws/transfers/test_mongo_to_s3.py
##########
@@ -25,7 +25,7 @@
 
 TASK_ID = 'test_mongo_to_s3_operator'
 MONGO_CONN_ID = 'default_mongo'
-S3_CONN_ID = 'default_s3'

Review comment:
       You removed S3_CONN_ID but you are referencing it in line 63

##########
File path: airflow/providers/amazon/aws/transfers/mongo_to_s3.py
##########
@@ -25,77 +26,101 @@
 from airflow.providers.mongo.hooks.mongo import MongoHook
 from airflow.utils.decorators import apply_defaults
 
+_DEPRECATION_MSG = (
+    "The s3_conn_id parameter has been deprecated. You should pass instead the aws_conn_id parameter."
+)
+
 
 class MongoToS3Operator(BaseOperator):
-    """
-    Mongo -> S3
-        A more specific baseOperator meant to move data
-        from mongo via pymongo to s3 via boto
-
-        things to note
-                .execute() is written to depend on .transform()
-                .transform() is meant to be extended by child classes
-                to perform transformations unique to those operators needs
+    """Operator meant to move data from mongo via pymongo to s3 via boto.
+
+    :param mongo_conn_id: reference to a specific mongo connection
+    :type mongo_conn_id: str
+    :param aws_conn_id: reference to a specific S3 connection
+    :type aws_conn_id: str
+    :param mongo_collection: reference to a specific collection in your mongo db
+    :type mongo_collection: str
+    :param mongo_query: query to execute. A list including a dict of the query
+    :type mongo_query: list
+    :param s3_bucket: reference to a specific S3 bucket to store the data
+    :type s3_bucket: str
+    :param s3_key: in which S3 key the file will be stored
+    :type s3_key: str
+    :param mongo_db: reference to a specific mongo database
+    :type mongo_db: str
+    :param replace: whether or not to replace the file in S3 if it previously existed
+    :param replace: bool
+    :param allow_disk_use: in the case you are retrieving a lot of data, you may have
+        to use the disk to save it instead of saving all in the RAM
+    :param allow_disk_use: bool
+    :param compression: Type of compression to use when saving the file in S3. Currently only gzip is supported.

Review comment:
       ```suggestion
       :param compression: type of compression to use for output file in S3. Currently only gzip is supported.
   ```
   You need line to be less than 110 chars. Yours has 112.
   Try this description with 107 chars or just use two lines like you do in lines 53-53 with the allow_disk_use param.

##########
File path: airflow/providers/amazon/aws/transfers/mongo_to_s3.py
##########
@@ -25,77 +26,101 @@
 from airflow.providers.mongo.hooks.mongo import MongoHook
 from airflow.utils.decorators import apply_defaults
 
+_DEPRECATION_MSG = (
+    "The s3_conn_id parameter has been deprecated. You should pass instead the aws_conn_id parameter."
+)
+
 
 class MongoToS3Operator(BaseOperator):
-    """
-    Mongo -> S3
-        A more specific baseOperator meant to move data
-        from mongo via pymongo to s3 via boto
-
-        things to note
-                .execute() is written to depend on .transform()
-                .transform() is meant to be extended by child classes
-                to perform transformations unique to those operators needs
+    """Operator meant to move data from mongo via pymongo to s3 via boto.
+
+    :param mongo_conn_id: reference to a specific mongo connection
+    :type mongo_conn_id: str
+    :param aws_conn_id: reference to a specific S3 connection
+    :type aws_conn_id: str
+    :param mongo_collection: reference to a specific collection in your mongo db
+    :type mongo_collection: str
+    :param mongo_query: query to execute. A list including a dict of the query
+    :type mongo_query: list
+    :param s3_bucket: reference to a specific S3 bucket to store the data
+    :type s3_bucket: str
+    :param s3_key: in which S3 key the file will be stored
+    :type s3_key: str
+    :param mongo_db: reference to a specific mongo database
+    :type mongo_db: str
+    :param replace: whether or not to replace the file in S3 if it previously existed
+    :param replace: bool
+    :param allow_disk_use: in the case you are retrieving a lot of data, you may have
+        to use the disk to save it instead of saving all in the RAM
+    :param allow_disk_use: bool

Review comment:
       ```suggestion
       :type allow_disk_use: bool
   ```

##########
File path: airflow/providers/amazon/aws/transfers/mongo_to_s3.py
##########
@@ -25,77 +26,101 @@
 from airflow.providers.mongo.hooks.mongo import MongoHook
 from airflow.utils.decorators import apply_defaults
 
+_DEPRECATION_MSG = (
+    "The s3_conn_id parameter has been deprecated. You should pass instead the aws_conn_id parameter."
+)
+
 
 class MongoToS3Operator(BaseOperator):
-    """
-    Mongo -> S3
-        A more specific baseOperator meant to move data
-        from mongo via pymongo to s3 via boto
-
-        things to note
-                .execute() is written to depend on .transform()
-                .transform() is meant to be extended by child classes
-                to perform transformations unique to those operators needs
+    """Operator meant to move data from mongo via pymongo to s3 via boto.
+
+    :param mongo_conn_id: reference to a specific mongo connection
+    :type mongo_conn_id: str
+    :param aws_conn_id: reference to a specific S3 connection
+    :type aws_conn_id: str
+    :param mongo_collection: reference to a specific collection in your mongo db
+    :type mongo_collection: str
+    :param mongo_query: query to execute. A list including a dict of the query
+    :type mongo_query: list
+    :param s3_bucket: reference to a specific S3 bucket to store the data
+    :type s3_bucket: str
+    :param s3_key: in which S3 key the file will be stored
+    :type s3_key: str
+    :param mongo_db: reference to a specific mongo database
+    :type mongo_db: str
+    :param replace: whether or not to replace the file in S3 if it previously existed
+    :param replace: bool

Review comment:
       ```suggestion
       :type replace: bool
   ```




----------------------------------------------------------------
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