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 2020/06/15 23:15:23 UTC

[GitHub] [airflow] potiuk opened a new pull request #9320: Introduce transfer packages

potiuk opened a new pull request #9320:
URL: https://github.com/apache/airflow/pull/9320


   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Target Github ISSUE in description if exists
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   


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



[GitHub] [airflow] potiuk commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440983246



##########
File path: docs/operators-and-hooks-ref.rst
##########
@@ -530,63 +530,63 @@ These integrations allow you to copy data from/to Amazon Web Services.
    * - `Amazon DynamoDB <https://aws.amazon.com/dynamodb/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.dynamodb_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.dynamodb_to_s3`
 
    * - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.redshift_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.redshift_to_s3`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - :doc:`How to use <howto/operator/amazon/aws/s3_to_redshift>`
-     - :mod:`airflow.providers.amazon.aws.operators.s3_to_redshift`
+     - :mod:`airflow.providers.amazon.aws.transfers.s3_to_redshift`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Snowflake <https://snowflake.com/>`__
      -
-     - :mod:`airflow.providers.snowflake.operators.s3_to_snowflake`
+     - :mod:`airflow.providers.snowflake.transfers.s3_to_snowflake`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Apache Hive <https://hive.apache.org/>`__
      -
-     - :mod:`airflow.providers.apache.hive.operators.s3_to_hive`
+     - :mod:`airflow.providers.apache.hive.transfers.s3_to_hive`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`__
      - `Google Cloud Storage (GCS) <https://cloud.google.com/gcs/>`__
      - :doc:`How to use <howto/operator/gcp/cloud_storage_transfer_service>`
-     - :mod:`airflow.providers.google.cloud.operators.s3_to_gcs`,
-       :mod:`airflow.providers.google.cloud.operators.cloud_storage_transfer_service`
+     - :mod:`airflow.providers.google.cloud.transfers.s3_to_gcs`,
+       :mod:`airflow.providers.google.cloud.transfers.cloud_storage_transfer_service`

Review comment:
       What do you mean by OK ? It looks good. What do you suspect is wrong here?
   




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



[GitHub] [airflow] potiuk merged pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #9320:
URL: https://github.com/apache/airflow/pull/9320


   


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



[GitHub] [airflow] feluelle commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440499396



##########
File path: UPDATING.md
##########
@@ -971,8 +971,8 @@ The following table shows changes in import paths.
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDatabaseUpdateOperator                         |airflow.providers.google.cloud.operators.spanner.SpannerUpdateDatabaseInstanceOperator                                        |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeleteOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeleteInstanceOperator                                                |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeployOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeployInstanceOperator                                                |
-|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.operators.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
-|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.operators.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |
+|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.transfers.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |

Review comment:
       That’s an exception. This should not be under transfer?! Transfer Operators are services which move data from one into another, correct?

##########
File path: UPDATING.md
##########
@@ -971,8 +971,8 @@ The following table shows changes in import paths.
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDatabaseUpdateOperator                         |airflow.providers.google.cloud.operators.spanner.SpannerUpdateDatabaseInstanceOperator                                        |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeleteOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeleteInstanceOperator                                                |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeployOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeployInstanceOperator                                                |
-|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.operators.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
-|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.operators.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |
+|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.transfers.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
+|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.transfers.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |

Review comment:
       Same here.




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



[GitHub] [airflow] feluelle commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r463044026



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       It calls [bulk_load_custom](https://github.com/apache/airflow/blob/68d1714f296989b7aad1a04b75dc033e76afb747/airflow/providers/mysql/hooks/mysql.py#L217) which uses [LOAD DATA LOCAL](https://dev.mysql.com/doc/refman/8.0/en/load-data-local-security.html)




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440866774



##########
File path: docs/operators-and-hooks-ref.rst
##########
@@ -530,63 +530,63 @@ These integrations allow you to copy data from/to Amazon Web Services.
    * - `Amazon DynamoDB <https://aws.amazon.com/dynamodb/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.dynamodb_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.dynamodb_to_s3`
 
    * - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.redshift_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.redshift_to_s3`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - :doc:`How to use <howto/operator/amazon/aws/s3_to_redshift>`
-     - :mod:`airflow.providers.amazon.aws.operators.s3_to_redshift`
+     - :mod:`airflow.providers.amazon.aws.transfers.s3_to_redshift`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Snowflake <https://snowflake.com/>`__
      -
-     - :mod:`airflow.providers.snowflake.operators.s3_to_snowflake`
+     - :mod:`airflow.providers.snowflake.transfers.s3_to_snowflake`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Apache Hive <https://hive.apache.org/>`__
      -
-     - :mod:`airflow.providers.apache.hive.operators.s3_to_hive`
+     - :mod:`airflow.providers.apache.hive.transfers.s3_to_hive`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`__
      - `Google Cloud Storage (GCS) <https://cloud.google.com/gcs/>`__
      - :doc:`How to use <howto/operator/gcp/cloud_storage_transfer_service>`
-     - :mod:`airflow.providers.google.cloud.operators.s3_to_gcs`,
-       :mod:`airflow.providers.google.cloud.operators.cloud_storage_transfer_service`
+     - :mod:`airflow.providers.google.cloud.transfers.s3_to_gcs`,
+       :mod:`airflow.providers.google.cloud.transfers.cloud_storage_transfer_service`

Review comment:
       Cna you check it?




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



[GitHub] [airflow] potiuk commented on pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#issuecomment-644872507


   @feluelle  - one more chance to take a look before I merge :)


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



[GitHub] [airflow] potiuk commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440508898



##########
File path: UPDATING.md
##########
@@ -971,8 +971,8 @@ The following table shows changes in import paths.
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDatabaseUpdateOperator                         |airflow.providers.google.cloud.operators.spanner.SpannerUpdateDatabaseInstanceOperator                                        |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeleteOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeleteInstanceOperator                                                |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeployOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeployInstanceOperator                                                |
-|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.operators.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
-|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.operators.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |
+|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.transfers.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
+|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.transfers.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |

Review comment:
       Ah yeah.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440854530



##########
File path: airflow/operators/google_api_to_s3_transfer.py
##########
@@ -17,32 +17,32 @@
 # under the License.
 """
 This module is deprecated.
-Please use `airflow.providers.amazon.aws.operators.google_api_to_s3_transfer`.
+Please use `airflow.providers.amazon.aws.transfers.google_api_to_s3_transfer`.
 """
 
 import warnings
 
-from airflow.providers.amazon.aws.operators.google_api_to_s3_transfer import GoogleApiToS3TransferOperator
+from airflow.providers.amazon.aws.transfers.google_api_to_s3_transfer import GoogleApiToS3Operator
 
 warnings.warn(
     "This module is deprecated. "
-    "Please use `airflow.providers.amazon.aws.operators.google_api_to_s3_transfer`.",
+    "Please use `airflow.providers.amazon.aws.transfers.google_api_to_s3_transfer`.",
     DeprecationWarning, stacklevel=2
 )
 
 
-class GoogleApiToS3Transfer(GoogleApiToS3TransferOperator):
+class GoogleApiToS3Transfer(GoogleApiToS3Operator):
     """
     This class is deprecated.
     Please use:
-    `airflow.providers.amazon.aws.operators.google_api_to_s3_transfer.GoogleApiToS3TransferOperator`."""
+    `airflow.providers.amazon.aws.transfers.google_api_to_s3_transfer.GoogleApiToS3Operator`."""
 
     def __init__(self, *args, **kwargs):
         warnings.warn(
             """This class is deprecated.
             Please use
             `airflow.providers.amazon.aws.operators.""" +

Review comment:
       ```suggestion
               `airflow.providers.amazon.aws.transfers.""" +
   ```




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



[GitHub] [airflow] potiuk commented on pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#issuecomment-644936278


   Added more descriptions for Transfer packages in CONTRIBUTING.rst and added transfer packages to be checked for consistency (added two missing modules and one missing howto.


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



[GitHub] [airflow] potiuk commented on pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#issuecomment-644761631


   > Can you view the file operators-and-hooks-ref.rst? It has sections with transfer operators for a long time, so it's easy to find a bug there.
   > Example:
   > :mod:`airflow.providers.amazon.aws.operators.sftp_to_s3` in Transfer operators and hooks
   > 
   > I think we should also add transfer package to check
   > https://github.com/PolideaInternal/airflow/blob/backport-packages-rename/docs/build#L171
   
   @mik-laj -> I think all the concerns are addressed now. I also reviewed operators-and-hooks.rst and it looks all should be fine now. 


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



[GitHub] [airflow] jensenity commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
jensenity commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r463350074



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       Thanks @feluelle, this helped a lot!




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



[GitHub] [airflow] potiuk commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440981294



##########
File path: airflow/providers/google/README.md
##########
@@ -421,7 +406,40 @@ All classes in Airflow 2.0 are in `airflow.providers.google` package.
 | [cloud.operators.vision.CloudVisionTextDetectOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                                   | [contrib.operators.gcp_vision_operator.CloudVisionDetectDocumentTextOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                            |
 | [cloud.operators.vision.CloudVisionUpdateProductOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                                | [contrib.operators.gcp_vision_operator.CloudVisionProductUpdateOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                                 |
 | [cloud.operators.vision.CloudVisionUpdateProductSetOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                             | [contrib.operators.gcp_vision_operator.CloudVisionProductSetUpdateOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                              |
-| [suite.operators.gcs_to_gdrive.GCSToGoogleDriveOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/suite/operators/gcs_to_gdrive.py)                                                          | [contrib.operators.gcs_to_gdrive_operator.GCSToGoogleDriveOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcs_to_gdrive_operator.py)                                                   |
+
+
+
+
+
+    ### New transfer operators
+
+    | New Airflow 2.0 transfers: `airflow.providers.google` package                                                                                                                       |
+|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| [ads.transfers.ads_to_gcs.GoogleAdsToGcsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/ads/transfers/ads_to_gcs.py)                               |
+| [cloud.transfers.facebook_ads_to_gcs.FacebookAdsReportToGcsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/facebook_ads_to_gcs.py) |
+| [cloud.transfers.gcs_to_local.GCSToLocalOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/gcs_to_local.py)                           |
+| [cloud.transfers.gcs_to_sftp.GCSToSFTPOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/gcs_to_sftp.py)                              |
+| [cloud.transfers.presto_to_gcs.PrestoToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/presto_to_gcs.py)                        |
+| [cloud.transfers.sftp_to_gcs.SFTPToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/sftp_to_gcs.py)                              |
+| [cloud.transfers.sheets_to_gcs.GoogleSheetsToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/sheets_to_gcs.py)                  |
+| [suite.transfers.gcs_to_sheets.GCSToGoogleSheetsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/suite/transfers/gcs_to_sheets.py)                  |
+
+
+
+    ### Moved transfer operators

Review comment:
       Nope. Fixed.




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



[GitHub] [airflow] feluelle commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r463048711



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       > The LOAD DATA statement reads rows from a text file into a table at a very high speed.
   
   You can define sth like:
   ```
       [{FIELDS | COLUMNS}
           [TERMINATED BY 'string']
           [[OPTIONALLY] ENCLOSED BY 'char']
           [ESCAPED BY 'char']
       ]
       [LINES
           [STARTING BY 'string']
           [TERMINATED BY 'string']
       ]
   ```
   for csv files.




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



[GitHub] [airflow] mik-laj commented on pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#issuecomment-644534662


   Can you view the file operators-and-hooks-ref.rst? It has sections with transfer operators for a long time, so it's easy to find a bug there.
   Example:
   :mod:`airflow.providers.amazon.aws.operators.sftp_to_s3` in Transfer operators and hooks
   
   I think we should also add transfer package to check
   https://github.com/PolideaInternal/airflow/blob/backport-packages-rename/docs/build#L171
   


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



[GitHub] [airflow] feluelle commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r463044026



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       It calls [bulk_load_custom](https://github.com/apache/airflow/blob/68d1714f296989b7aad1a04b75dc033e76afb747/airflow/providers/mysql/hooks/mysql.py#L217) which uses [LOAD DATA](https://dev.mysql.com/doc/refman/8.0/en/load-data.html)




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



[GitHub] [airflow] jensenity commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
jensenity commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r464212247



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       One more question! Does this work with parquet files? After looking into the docs, I don't think it does. Right?




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



[GitHub] [airflow] feluelle commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r464477157



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       No I don't think it works directly. You would have to convert parquet to csv and load csv to mysql.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r441003046



##########
File path: docs/operators-and-hooks-ref.rst
##########
@@ -530,63 +530,63 @@ These integrations allow you to copy data from/to Amazon Web Services.
    * - `Amazon DynamoDB <https://aws.amazon.com/dynamodb/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.dynamodb_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.dynamodb_to_s3`
 
    * - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.redshift_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.redshift_to_s3`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - :doc:`How to use <howto/operator/amazon/aws/s3_to_redshift>`
-     - :mod:`airflow.providers.amazon.aws.operators.s3_to_redshift`
+     - :mod:`airflow.providers.amazon.aws.transfers.s3_to_redshift`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Snowflake <https://snowflake.com/>`__
      -
-     - :mod:`airflow.providers.snowflake.operators.s3_to_snowflake`
+     - :mod:`airflow.providers.snowflake.transfers.s3_to_snowflake`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Apache Hive <https://hive.apache.org/>`__
      -
-     - :mod:`airflow.providers.apache.hive.operators.s3_to_hive`
+     - :mod:`airflow.providers.apache.hive.transfers.s3_to_hive`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`__
      - `Google Cloud Storage (GCS) <https://cloud.google.com/gcs/>`__
      - :doc:`How to use <howto/operator/gcp/cloud_storage_transfer_service>`
-     - :mod:`airflow.providers.google.cloud.operators.s3_to_gcs`,
-       :mod:`airflow.providers.google.cloud.operators.cloud_storage_transfer_service`
+     - :mod:`airflow.providers.google.cloud.transfers.s3_to_gcs`,
+       :mod:`airflow.providers.google.cloud.transfers.cloud_storage_transfer_service`

Review comment:
       Cloud transfer service operators should be in operators package. 




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



[GitHub] [airflow] potiuk commented on pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#issuecomment-644872232


   @mik-laj -> all corrected, just the last comment I am not sure about 


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



[GitHub] [airflow] jensenity commented on a change in pull request #9320: Introduce 'transfers' packages

Posted by GitBox <gi...@apache.org>.
jensenity commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r462909246



##########
File path: airflow/providers/mysql/transfers/s3_to_mysql.py
##########
@@ -24,7 +24,7 @@
 from airflow.utils.decorators import apply_defaults
 
 
-class S3ToMySqlTransferOperator(BaseOperator):
+class S3ToMySqlOperator(BaseOperator):
     """
     Loads a file from S3 into a MySQL table.

Review comment:
       Can I ask what's the requirement S3 file format for this Operator? json? csv? parquet and etc.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440866774



##########
File path: docs/operators-and-hooks-ref.rst
##########
@@ -530,63 +530,63 @@ These integrations allow you to copy data from/to Amazon Web Services.
    * - `Amazon DynamoDB <https://aws.amazon.com/dynamodb/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.dynamodb_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.dynamodb_to_s3`
 
    * - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      -
-     - :mod:`airflow.providers.amazon.aws.operators.redshift_to_s3`
+     - :mod:`airflow.providers.amazon.aws.transfers.redshift_to_s3`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Amazon Redshift <https://aws.amazon.com/redshift/>`__
      - :doc:`How to use <howto/operator/amazon/aws/s3_to_redshift>`
-     - :mod:`airflow.providers.amazon.aws.operators.s3_to_redshift`
+     - :mod:`airflow.providers.amazon.aws.transfers.s3_to_redshift`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Snowflake <https://snowflake.com/>`__
      -
-     - :mod:`airflow.providers.snowflake.operators.s3_to_snowflake`
+     - :mod:`airflow.providers.snowflake.transfers.s3_to_snowflake`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`_
      - `Apache Hive <https://hive.apache.org/>`__
      -
-     - :mod:`airflow.providers.apache.hive.operators.s3_to_hive`
+     - :mod:`airflow.providers.apache.hive.transfers.s3_to_hive`
 
    * - `Amazon Simple Storage Service (S3) <https://aws.amazon.com/s3/>`__
      - `Google Cloud Storage (GCS) <https://cloud.google.com/gcs/>`__
      - :doc:`How to use <howto/operator/gcp/cloud_storage_transfer_service>`
-     - :mod:`airflow.providers.google.cloud.operators.s3_to_gcs`,
-       :mod:`airflow.providers.google.cloud.operators.cloud_storage_transfer_service`
+     - :mod:`airflow.providers.google.cloud.transfers.s3_to_gcs`,
+       :mod:`airflow.providers.google.cloud.transfers.cloud_storage_transfer_service`

Review comment:
       Cna you check it? Is it ok?




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



[GitHub] [airflow] potiuk commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440794930



##########
File path: UPDATING.md
##########
@@ -971,8 +971,8 @@ The following table shows changes in import paths.
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDatabaseUpdateOperator                         |airflow.providers.google.cloud.operators.spanner.SpannerUpdateDatabaseInstanceOperator                                        |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeleteOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeleteInstanceOperator                                                |
 |airflow.contrib.operators.gcp_spanner_operator.CloudSpannerInstanceDeployOperator                                 |airflow.providers.google.cloud.operators.spanner.SpannerDeployInstanceOperator                                                |
-|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.operators.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |
-|airflow.contrib.operators.gcp_text_to_speech_operator.GcpTextToSpeechSynthesizeOperator                           |airflow.providers.google.cloud.operators.text_to_speech.CloudTextToSpeechSynthesizeOperator                                   |
+|airflow.contrib.operators.gcp_speech_to_text_operator.GcpSpeechToTextRecognizeSpeechOperator                      |airflow.providers.google.cloud.transfers.speech_to_text.CloudSpeechToTextRecognizeSpeechOperator                              |

Review comment:
       Yep. REsolved




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440586097



##########
File path: CONTRIBUTING.rst
##########
@@ -542,8 +542,48 @@ We support the following types of tests:
 
 For details on running different types of Airflow tests, see `TESTING.rst <TESTING.rst>`_.
 
+
+Naming Conventions for provider packages
+========================================
+
+In Airflow 2.0 we standardized and enforced naming for provider packages, modules and classes.
+those rules (introduced as AIP-21) were not only introduced but enforced using automated checks
+that verify if the naming conventions are followed. Here is a brief summary of the rules, for
+detailed discussion you can go to [AIP-21 Changes in import paths](https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths)
+
+The rules are as follows:
+
+* Provider packages are all placed in 'airflow.providers'
+* Providers are usually direct sub-packages of the 'airflow.providers' package but in some cases they can be
+  further split into sub-packages (for example 'apache' package has 'cassandra', 'druid' ... providers ) out
+  of which several different provider packages are produced (apache.cassandra, apache.druid). This is
+  case when the providers are connected under common umbrella but very loosely coupled on the code level.
+* In some cases the package can have sub-packages but they are all delivered as single provider
+  package (for example 'google' package contains 'ads', 'cloud' etc. sub-packages). This is in case
+  the providers are connected under common umbrella and they are also tightly coupled on the code level.
+* Typical structure of provider package:
+    * example_dags -> example DAGs are stored here (used for documentation and System Tests)
+    * hooks -> hooks are stored here
+    * operators -> operators are stored here
+    * sensors -> sensors are stored here
+    * secrets -> secret backends are stored here
+    * transfers -> transfer operators are stored here
+* Module names do not contain word "hooks" , "operators" etc. The right type comes from
+  the package. For example 'hooks.datastore' module contains DataStore hook and 'operators.datastore'
+  contains DataStore operators.
+* Class names contain 'Operator', 'Hook', 'Sensor' - for example DataStoreHook, DataStoreExportOperator
+* Operator name usually follows the convention: <Subject><Action>Operator (DataStoreExportOperator) is a good

Review comment:
       ```suggestion
   * Operator name usually follows the convention: <Subject><Action><Entity>Operator (BigQueryExecuteQueryOperator) is a good
   ```
   It is not a good idea to use the verb alone. Here is an example: https://github.com/apache/airflow/pull/9314
   We had CloudBuildCreate to create Build, and then it turned out that the service has grown and you can also create BuildTriggey, so we created a new operator CloudBuildCreateBuildTrigger. If we do not have a noun, the operator's name has become ambiguous.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #9320: Introduce transfer packages

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #9320:
URL: https://github.com/apache/airflow/pull/9320#discussion_r440859035



##########
File path: airflow/providers/google/README.md
##########
@@ -421,7 +406,40 @@ All classes in Airflow 2.0 are in `airflow.providers.google` package.
 | [cloud.operators.vision.CloudVisionTextDetectOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                                   | [contrib.operators.gcp_vision_operator.CloudVisionDetectDocumentTextOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                            |
 | [cloud.operators.vision.CloudVisionUpdateProductOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                                | [contrib.operators.gcp_vision_operator.CloudVisionProductUpdateOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                                 |
 | [cloud.operators.vision.CloudVisionUpdateProductSetOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/operators/vision.py)                                                             | [contrib.operators.gcp_vision_operator.CloudVisionProductSetUpdateOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcp_vision_operator.py)                                              |
-| [suite.operators.gcs_to_gdrive.GCSToGoogleDriveOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/suite/operators/gcs_to_gdrive.py)                                                          | [contrib.operators.gcs_to_gdrive_operator.GCSToGoogleDriveOperator](https://github.com/apache/airflow/blob/v1-10-stable/airflow/contrib/operators/gcs_to_gdrive_operator.py)                                                   |
+
+
+
+
+
+    ### New transfer operators
+
+    | New Airflow 2.0 transfers: `airflow.providers.google` package                                                                                                                       |
+|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| [ads.transfers.ads_to_gcs.GoogleAdsToGcsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/ads/transfers/ads_to_gcs.py)                               |
+| [cloud.transfers.facebook_ads_to_gcs.FacebookAdsReportToGcsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/facebook_ads_to_gcs.py) |
+| [cloud.transfers.gcs_to_local.GCSToLocalOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/gcs_to_local.py)                           |
+| [cloud.transfers.gcs_to_sftp.GCSToSFTPOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/gcs_to_sftp.py)                              |
+| [cloud.transfers.presto_to_gcs.PrestoToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/presto_to_gcs.py)                        |
+| [cloud.transfers.sftp_to_gcs.SFTPToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/sftp_to_gcs.py)                              |
+| [cloud.transfers.sheets_to_gcs.GoogleSheetsToGCSOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/transfers/sheets_to_gcs.py)                  |
+| [suite.transfers.gcs_to_sheets.GCSToGoogleSheetsOperator](https://github.com/apache/airflow/blob/master/airflow/providers/google/suite/transfers/gcs_to_sheets.py)                  |
+
+
+
+    ### Moved transfer operators

Review comment:
       Is this indentation expected?




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