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/03/10 19:53:49 UTC

[GitHub] [airflow] swiftomkar opened a new pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

swiftomkar opened a new pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688
 
 
   ---
   Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
   
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
   - [ ] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [ ] 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).
   
   <sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
   
   ---
   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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391051551
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   Yes. It is not breaking changes. 

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r400921868
 
 

 ##########
 File path: tests/providers/amazon/aws/operators/test_redshift_to_s3.py
 ##########
 @@ -76,3 +76,55 @@ def test_execute(self, table_as_file_name, expected_s3_key, mock_run, mock_sessi
 
         assert mock_run.call_count == 1
         assert_equal_ignore_multiple_spaces(self, mock_run.call_args[0][0], unload_query)
+
+    @parameterized.expand([
+        [None],
+        ["SELECT * FROM schema.table;"],
+    ])
+    @mock.patch("boto3.session.Session")
+    @mock.patch("airflow.providers.postgres.hooks.postgres.PostgresHook.run")
+    def test_transfer_customquery_success(self, expected_custom_select_query, mock_run, mock_session,):
 
 Review comment:
   ```suggestion
       def test_transfer_customquery_success(self, expected_custom_select_query, mock_run, mock_session):
   ```

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391046137
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements. This should be followed by either `schema` and `table` and table arguements or a single `custom_select_query` arguement. 
+The `schema` and `table` arg combo unloads everything from the given table into an s3 object while `custom_select_query` allows you to specify your own redshift select query to filter data on and unload to s3. 
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table', 's3_bucket', 's3_key')  # Pre-2.0 call
 
 Review comment:
   Got 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


With regards,
Apache Git Services

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r401008701
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
 
 Review comment:
   Not really

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390914859
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if all([self.schema, self.table]):
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        elif self.custom_select_query:
+            select_query = self.custom_select_query
+        else:
+            raise AirflowBadRequest(f"Either (schema, table) or custom_select_query should be set. They are ({self.schema},{self.table}) and {self.custom_select_query} now.")
 
 Review comment:
   This validation should be made during operator `__init__` (or at least before initiating the hooks).

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390591494
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -72,10 +75,11 @@ class RedshiftToS3Transfer(BaseOperator):
     @apply_defaults
     def __init__(  # pylint: disable=too-many-arguments
             self,
-            schema: str,
-            table: str,
             s3_bucket: str,
             s3_key: str,
+            schema: str = None,
 
 Review comment:
   This is a breaking change so it should be described in UPDATING.md. It's ok for 2.0 (we allow such backwards incompatibilities) but it must be described there.

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r402389214
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
 
 Review comment:
   Please use `and`. `all` requires a list which you do not need if you use `and`.

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r402203167
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
 
 Review comment:
   I think `all` makes it cleaner.

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390719168
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1048,21 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+## Airflow 2.0
 
 Review comment:
   @potiuk Got it! Thanks! Can you take a look to see if this looks good?

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

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390705298
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
+            if self.schema is None or self.table is None:
 
 Review comment:
   ```suggestion
               if (not self.schema) or (not self.table):
   ```

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390623978
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1048,21 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+## Airflow 2.0
 
 Review comment:
   It's actually Airflow Master :)

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391071044
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,20 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements.
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table')  # Pre-2.0 call
+...
+result = RedshiftToS3Transfer('s3_bucket', 's3_key')  # Post-2.0 call
 
 Review comment:
   Removed documentation from UPDATING.md

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

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391052165

##########
File path: UPDATING.md
##########
@@ -1047,6 +1047,20 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip

The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).

Review comment:
We should use keywords arguments only for operators.

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390915364

##########
File path: UPDATING.md
##########
@@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip

Review comment:
```suggestion
the `s3_bucket` and `s3_key` are the first 2 positional arguments. This should be followed by either `schema` and `table` and table arguements or a single `custom_select_query` arguement.
```

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390751106

##########
File path: UPDATING.md
##########
@@ -1047,6 +1047,20 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip

Review comment:
I think the examples are not correct and it might be misleading - the first one should also have s3_bucket and s3_key -> they were not optional. Also the second example is not valid because it requires either schema+table combination or custom query.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597447171
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=h1) Report
   > Merging [#7688](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/85465499074443fa98322051ca11f43c66242cc3&el=desc) will **decrease** coverage by `0.78%`.
   > The diff coverage is `85.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7688/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7688      +/-   ##
   ==========================================
   - Coverage   86.83%   86.05%   -0.79%     
   ==========================================
     Files         897      904       +7     
     Lines       42803    43742     +939     
   ==========================================
   + Hits        37170    37643     +473     
   - Misses       5633     6099     +466     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...w/providers/amazon/aws/operators/redshift\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvcmVkc2hpZnRfdG9fczMucHk=) | `95.34% <85.71%> (-1.95%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0.00%> (-54.93%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
   | ... and [37 more](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=footer). Last update [8546549...f6fdf04](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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

[GitHub] [airflow] codecov-io commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

codecov-io commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597447171
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=h1) Report
   > Merging [#7688](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/85465499074443fa98322051ca11f43c66242cc3&el=desc) will **decrease** coverage by `0.17%`.
   > The diff coverage is `85.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7688/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7688      +/-   ##
   ==========================================
   - Coverage   86.83%   86.66%   -0.18%     
   ==========================================
     Files         897      904       +7     
     Lines       42803    43742     +939     
   ==========================================
   + Hits        37170    37909     +739     
   - Misses       5633     5833     +200     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...w/providers/amazon/aws/operators/redshift\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvcmVkc2hpZnRfdG9fczMucHk=) | `95.34% <85.71%> (-1.95%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0.00%> (-45.66%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [...viders/cncf/kubernetes/operators/kubernetes\_pod.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL29wZXJhdG9ycy9rdWJlcm5ldGVzX3BvZC5weQ==) | `69.69% <0.00%> (-25.26%)` | :arrow_down: |
   | [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0.00%> (-23.53%)` | :arrow_down: |
   | [...rflow/providers/google/cloud/operators/dataflow.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9kYXRhZmxvdy5weQ==) | `90.44% <0.00%> (-8.65%)` | :arrow_down: |
   | [airflow/jobs/scheduler\_job.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL3NjaGVkdWxlcl9qb2IucHk=) | `90.53% <0.00%> (-0.12%)` | :arrow_down: |
   | [airflow/providers/mysql/hooks/mysql.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvaG9va3MvbXlzcWwucHk=) | `92.70% <0.00%> (-0.07%)` | :arrow_down: |
   | ... and [21 more](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=footer). Last update [8546549...98f875f](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391044835
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements. This should be followed by either `schema` and `table` and table arguements or a single `custom_select_query` arguement. 
+The `schema` and `table` arg combo unloads everything from the given table into an s3 object while `custom_select_query` allows you to specify your own redshift select query to filter data on and unload to s3. 
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table', 's3_bucket', 's3_key')  # Pre-2.0 call
 
 Review comment:
   I'm sorry, I didn't exactly understand. Could you explain?

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390599830
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
 
 Review comment:
   Yes, I will work on adding the case, Thank you!

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

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390993009
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   The order of the arguments does not matter in operators. Each operator must use named arguments, because we have ``apply_default`` decorator.

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r400920941
 
 

 ##########
 File path: tests/providers/amazon/aws/operators/test_redshift_to_s3.py
 ##########
 @@ -76,3 +76,55 @@ def test_execute(self, table_as_file_name, expected_s3_key, mock_run, mock_sessi
 
         assert mock_run.call_count == 1
         assert_equal_ignore_multiple_spaces(self, mock_run.call_args[0][0], unload_query)
+
+    @parameterized.expand([
+        [None],
+        ["SELECT * FROM schema.table;"],
+    ])
+    @mock.patch("boto3.session.Session")
+    @mock.patch("airflow.providers.postgres.hooks.postgres.PostgresHook.run")
+    def test_transfer_customquery_success(self, expected_custom_select_query, mock_run, mock_session,):
+        access_key = "aws_access_key_id"
+        secret_key = "aws_secret_access_key"
+        mock_session.return_value = Session(access_key, secret_key)
+        custom_select_query = "SELECT * FROM schema.table;"
+        schema = "schema"
+        table = "table"
+        s3_bucket = "bucket"
+        s3_key = "key"
+        unload_options = ['HEADER', ]
+
+        RedshiftToS3Transfer(
+            custom_select_query=custom_select_query,
+            schema=schema,
+            table=table,
+            s3_bucket=s3_bucket,
+            s3_key=s3_key,
+            unload_options=unload_options,
+            include_header=True,
+            redshift_conn_id="redshift_conn_id",
+            aws_conn_id="aws_conn_id",
+            task_id="task_id",
+            dag=None
+        ).execute(None)
+
+        unload_options = '\n\t\t\t'.join(unload_options)
+        if expected_custom_select_query:
+            select_query = expected_custom_select_query
+        else:
+            select_query = "SELECT * FROM schema.table;"
 
 Review comment:
   ```suggestion
           select_query = expected_custom_select_query or "SELECT * FROM schema.table;"
   ```

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

[GitHub] [airflow] zhongjiajie commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-608184565
 
 
   CI failed due to unused param
   ```sh
   ************* Module tests.providers.amazon.aws.operators.test_redshift_to_s3
   tests/providers/amazon/aws/operators/test_redshift_to_s3.py:136:36: W0613: Unused argument 'expected_schema' (unused-argument)
   tests/providers/amazon/aws/operators/test_redshift_to_s3.py:136:53: W0613: Unused argument 'expected_table' (unused-argument)
   ```

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

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390705128
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
 
 Review comment:
   ```suggestion
           if not self.custom_select_query:
   ```

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

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391051551
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   Yes. It is not breaking change.

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

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390707943
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1048,21 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+## Airflow 2.0
 
 Review comment:
   BTW, should we put the latest change in the top of UPDATING.md @potiuk 

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391045710
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   I see! So does this mean that this is not a breaking change @mik-laj ?

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

[GitHub] [airflow] swiftomkar commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597417616
 
 
   @zhongjiajie Thanks for your suggestions!

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r400918090
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
 
 Review comment:
   ```suggestion
           if self.schema and self.table:
   ```
   Is `all` really necessary?

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390593284
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
 
 Review comment:
   We need  a test case covering that case. there is a test already in TestRedshiftToS3Transfer and you can add new one with custom select query 

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r400924254
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        elif self.custom_select_query:
+            select_query = self.custom_select_query
+        else:
+            raise AirflowBadRequest(f"""Either (schema, table) or custom_select_query should be set.
+            They are ({self.schema},{self.table}) and {self.custom_select_query} now.""")
 
 Review comment:
   ```suggestion
               raise AirflowBadRequest("Either (schema, table) or custom_select_query should be set. "
                   f"They are ({self.schema},{self.table}) and {self.custom_select_query} now.")
   ```
   If you use triple quotes `"""` it will also print the spaces you have in front of `They`. And in this case you probably do not want that.

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391036023
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if all([self.schema, self.table]):
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        elif self.custom_select_query:
+            select_query = self.custom_select_query
+        else:
+            raise AirflowBadRequest(f"Either (schema, table) or custom_select_query should be set. They are ({self.schema},{self.table}) and {self.custom_select_query} now.")
 
 Review comment:
   Makes sense. I will make the change. Thanks!

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391044835
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements. This should be followed by either `schema` and `table` and table arguements or a single `custom_select_query` arguement. 
+The `schema` and `table` arg combo unloads everything from the given table into an s3 object while `custom_select_query` allows you to specify your own redshift select query to filter data on and unload to s3. 
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table', 's3_bucket', 's3_key')  # Pre-2.0 call
 
 Review comment:
   I'm sorry, I didn't exactly understand. Could you explain?

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

[GitHub] [airflow] codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597447171
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=h1) Report
   > Merging [#7688](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/85465499074443fa98322051ca11f43c66242cc3?src=pr&el=desc) will **decrease** coverage by `0.78%`.
   > The diff coverage is `85.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7688/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7688      +/-   ##
   ==========================================
   - Coverage   86.83%   86.05%   -0.79%     
   ==========================================
     Files         897      904       +7     
     Lines       42803    43742     +939     
   ==========================================
   + Hits        37170    37643     +473     
   - Misses       5633     6099     +466
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...w/providers/amazon/aws/operators/redshift\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvcmVkc2hpZnRfdG9fczMucHk=) | `95.34% <85.71%> (-1.95%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0%> (-54.93%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0%> (-46.67%)` | :arrow_down: |
   | [airflow/security/kerberos.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZWN1cml0eS9rZXJiZXJvcy5weQ==) | `30.43% <0%> (-45.66%)` | :arrow_down: |
   | ... and [37 more](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=footer). Last update [8546549...f6fdf04](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390591494
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -72,10 +75,11 @@ class RedshiftToS3Transfer(BaseOperator):
     @apply_defaults
     def __init__(  # pylint: disable=too-many-arguments
             self,
-            schema: str,
-            table: str,
             s3_bucket: str,
             s3_key: str,
+            schema: str = None,
 
 Review comment:
   This is a breaking change  (someone could have used those as positional parameters) so it should be described in UPDATING.md. It's ok for 2.0 (we allow such backwards incompatibilities) but it must be described there.

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390754114
 
 

 ##########
 File path: tests/providers/amazon/aws/operators/test_redshift_to_s3.py
 ##########
 @@ -76,3 +76,49 @@ def test_execute(self, table_as_file_name, expected_s3_key, mock_run, mock_sessi
 
         assert mock_run.call_count == 1
         assert_equal_ignore_multiple_spaces(self, mock_run.call_args[0][0], unload_query)
+
+    @parameterized.expand([
+        [True, "key/None_"],
+        [False, "key"],
 
 Review comment:
   I like that you used parameterized. But we need to add custom_select_query to parameterized parameters and also we need to handle the failure cases here (with various combinations of None parameters set - just table None, just schema None, all None etc so the parameterized should have the "expected exception" also (or maybe we should have failure test cases as separate method - both will work.

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

[GitHub] [airflow] codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597447171
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=h1) Report
   > Merging [#7688](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/85465499074443fa98322051ca11f43c66242cc3?src=pr&el=desc) will **decrease** coverage by `25.62%`.
   > The diff coverage is `64.57%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7688/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #7688       +/-   ##
   ===========================================
   - Coverage   86.83%   61.21%   -25.63%     
   ===========================================
     Files         897      915       +18     
     Lines       42803    44182     +1379     
   ===========================================
   - Hits        37170    27048    -10122     
   - Misses       5633    17134    +11501
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/jobs/base\_job.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2Jhc2Vfam9iLnB5) | `91.54% <ø> (ø)` | :arrow_up: |
   | [airflow/example\_dags/subdags/subdag.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3Mvc3ViZGFncy9zdWJkYWcucHk=) | `100% <ø> (ø)` | :arrow_up: |
   | [airflow/example\_dags/example\_subdag\_operator.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9zdWJkYWdfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
   | [...ders/google/cloud/example\_dags/example\_dataflow.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2V4YW1wbGVfZGFncy9leGFtcGxlX2RhdGFmbG93LnB5) | `100% <ø> (ø)` | :arrow_up: |
   | [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `94.36% <ø> (-0.71%)` | :arrow_down: |
   | [airflow/providers/google/cloud/operators/pubsub.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9wdWJzdWIucHk=) | `74.41% <ø> (-16.28%)` | :arrow_down: |
   | [airflow/www/utils.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdXRpbHMucHk=) | `79.39% <ø> (-0.99%)` | :arrow_down: |
   | [airflow/providers/openfaas/hooks/openfaas.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvb3BlbmZhYXMvaG9va3Mvb3BlbmZhYXMucHk=) | `29.41% <ø> (ø)` | |
   | [...w/example\_dags/example\_latest\_only\_with\_trigger.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9sYXRlc3Rfb25seV93aXRoX3RyaWdnZXIucHk=) | `100% <ø> (ø)` | :arrow_up: |
   | [airflow/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9fX2luaXRfXy5weQ==) | `100% <ø> (+8%)` | :arrow_up: |
   | ... and [383 more](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=footer). Last update [8546549...4eb6c5c](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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

[GitHub] [airflow] zhongjiajie commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-599833514
 
 
   @swiftomkar FYI, CI failed

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

[GitHub] [airflow] codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#issuecomment-597447171
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=h1) Report
   > Merging [#7688](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/85465499074443fa98322051ca11f43c66242cc3&el=desc) will **decrease** coverage by `0.62%`.
   > The diff coverage is `85.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7688/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7688      +/-   ##
   ==========================================
   - Coverage   86.83%   86.21%   -0.63%     
   ==========================================
     Files         897      904       +7     
     Lines       42803    43742     +939     
   ==========================================
   + Hits        37170    37710     +540     
   - Misses       5633     6032     +399     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...w/providers/amazon/aws/operators/redshift\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvcmVkc2hpZnRfdG9fczMucHk=) | `95.34% <85.71%> (-1.95%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
   | ... and [34 more](https://codecov.io/gh/apache/airflow/pull/7688/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=footer). Last update [8546549...f6fdf04](https://codecov.io/gh/apache/airflow/pull/7688?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

##########
File path: UPDATING.md
##########
@@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip

+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements. This should be followed by either `schema` and `table` and table arguements or a single `custom_select_query` arguement.
+The `schema` and `table` arg combo unloads everything from the given table into an s3 object while `custom_select_query` allows you to specify your own redshift select query to filter data on and unload to s3.
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table', 's3_bucket', 's3_key') # Pre-2.0 call

Review comment:
This code is invalid allways.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390707156
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
+            if self.schema is None or self.table is None:
+                raise AirflowBadRequest("schema, table set to None")
 
 Review comment:
   I think is better to use `Both custom_select_query and (schema, table) be None not allow here` or some else, you could not just hint with `schema` and `table`

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390753208
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if not self.custom_select_query:
+            if (not self.schema) or (not self.table):
+                raise AirflowBadRequest("schema, table set to None")
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        else:
+            select_query = self.custom_select_query
 
 Review comment:
   ```suggestion
           if all([self.schema, self.table]):
                 select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
           elif self.custom_select_query:
                 select_query = self.custom_select_query
           else:
                raise AirflowBadRequest(f"Either (schema, table) or custom_select_query should be set. They are ({self.schema},{self.table}) and {self.custom_select_query} 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


With regards,
Apache Git Services

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391059082
 
 

 ##########
 File path: tests/providers/amazon/aws/operators/test_redshift_to_s3.py
 ##########
 @@ -76,3 +76,49 @@ def test_execute(self, table_as_file_name, expected_s3_key, mock_run, mock_sessi
 
         assert mock_run.call_count == 1
         assert_equal_ignore_multiple_spaces(self, mock_run.call_args[0][0], unload_query)
+
+    @parameterized.expand([
+        [True, "key/None_"],
+        [False, "key"],
 
 Review comment:
   @potiuk Got it! Will work on it 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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

mik-laj commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391064247
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   It does not require a note in UPDATING.md.

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391035372
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,20 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
+positional arguments. This signature was changed in 2.0 and
+the `s3_bucket` and `s3_key` are the first 2 positional reguements.
+
+In order to use this operator:
+```python
+result = RedshiftToS3Transfer('schema', 'table')  # Pre-2.0 call
+...
+result = RedshiftToS3Transfer('s3_bucket', 's3_key')  # Post-2.0 call
 
 Review comment:
   I've tried fixing it...how does it look 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


With regards,
Apache Git Services

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r391056490
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1047,24 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+
+### RedshiftToS3Transfer:: signature changed
+
+Previous versions of the `RedshiftToS3Transfer` operator required `schema` and `table` arguments as the first 2
 
 Review comment:
   Ah! understood. So I am assuming that there is no need to document this in UPDATING.md. @mik-laj  

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390719168
 
 

 ##########
 File path: UPDATING.md
 ##########
 @@ -1047,6 +1048,21 @@ If the DAG relies on tasks with other trigger rules (i.e. `all_done`) being skip
 
 The goal of this change is to achieve a more consistent and configurale cascading behaviour based on the `BaseBranchOperator` (see [AIRFLOW-2923](https://jira.apache.org/jira/browse/AIRFLOW-2923) and [AIRFLOW-1784](https://jira.apache.org/jira/browse/AIRFLOW-1784)).
 
+## Airflow 2.0
 
 Review comment:
   @potiuk Got it! Thanks!

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

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390752987
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if self.custom_select_query is None:
+            if self.schema is None or self.table is None:
+                raise AirflowBadRequest("schema, table set to None")
 
 Review comment:
   @swiftomkar WDYT, about this one?

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

[GitHub] [airflow] swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

swiftomkar commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390602548
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -72,10 +75,11 @@ class RedshiftToS3Transfer(BaseOperator):
     @apply_defaults
     def __init__(  # pylint: disable=too-many-arguments
             self,
-            schema: str,
-            table: str,
             s3_bucket: str,
             s3_key: str,
+            schema: str = None,
 
 Review comment:
   I see! I will document it in UPDATING.md 

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

[GitHub] [airflow] feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

feluelle commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r400917262
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -101,13 +106,19 @@ def __init__(  # pylint: disable=too-many-arguments
             self.unload_options = list(self.unload_options) + ['HEADER', ]
 
     def execute(self, context):
+        if all([self.schema, self.table]):
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
 
 Review comment:
   ```suggestion
               select_query = f"SELECT * FROM {self.schema}.{self.table}"
   ```

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

[GitHub] [airflow] potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query

Posted by GitBox <gi...@apache.org>.

potiuk commented on a change in pull request #7688: [AIRFLOW-6794] Allow AWS Operator RedshiftToS3Transfer To Run a Custom Query
URL: https://github.com/apache/airflow/pull/7688#discussion_r390753368
 
 

 ##########
 File path: airflow/providers/amazon/aws/operators/redshift_to_s3.py
 ##########
 @@ -107,7 +112,12 @@ def execute(self, context):
         credentials = s3_hook.get_credentials()
         unload_options = '\n\t\t\t'.join(self.unload_options)
         s3_key = '{}/{}_'.format(self.s3_key, self.table) if self.table_as_file_name else self.s3_key
-        select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        if not self.custom_select_query:
+            if (not self.schema) or (not self.table):
+                raise AirflowBadRequest("schema, table set to None")
+            select_query = "SELECT * FROM {schema}.{table}".format(schema=self.schema, table=self.table)
+        else:
+            select_query = self.custom_select_query
 
 Review comment:
   That would be more pythonic.

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