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/12/11 14:56:37 UTC
[GitHub] [airflow] davido912 opened a new pull request #13017: Do/12001 snowflake schema
davido912 opened a new pull request #13017:
URL: https://github.com/apache/airflow/pull/13017
closing 12001.
schema can now be optionally passed, either via a parameter or via connection metadata.
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-743265890
[The Workflow run](https://github.com/apache/airflow/actions/runs/415709251) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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] dstandish commented on a change in pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#discussion_r549970545
##########
File path: airflow/providers/snowflake/transfers/s3_to_snowflake.py
##########
@@ -91,16 +91,18 @@ def execute(self, context: Any) -> None:
)
if self.columns_array:
- copy_query = """
- COPY INTO {schema}.{table}({columns}) {base_sql}
- """.format(
- schema=self.schema, table=self.table, columns=",".join(self.columns_array), base_sql=base_sql
+ columns = ','.join(self.columns_array)
+ copy_query = (
+ f"COPY INTO {self.schema}.{self.table}({columns}) {base_sql}"
+ if self.schema
+ else f"COPY INTO {self.table} ({columns}) {base_sql}"
)
+
else:
Review comment:
```suggestion
columns = '(' + ','.join(self.columns_array) + ')' if self.columns_array else ''
table_name = f"{self.schema}.{self.table}" if self.schema else self.table
copy_query = f"COPY INTO {table_name} {columns} {from_sql}"
```
this whole if / else block could be condensed to this (but the platform isn't letting me apply the suggestion correctly)
even if maybe you don't go with my exact suggestion, it's just more verbose than it needs to be ( e.g. COPY INTO appears 4 times) and could be cleaned up
note here i'm also suggesting renaming `base_sql` to `from_sql` because that's more descriptive -- it's the FROM clause
----------------------------------------------------------------
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] github-actions[bot] closed pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #13017:
URL: https://github.com/apache/airflow/pull/13017
--
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] github-actions[bot] commented on pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-749231745
[The Workflow run](https://github.com/apache/airflow/actions/runs/436774410) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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] davido912 commented on pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
davido912 commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-749419787
@potiuk This should be fine to merge. I've tested it on an earlier Airflow version but not yet 2.0, do you figure that would be necessary? Don't think there should be any difference.
The normal CI checks have all passed
----------------------------------------------------------------
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 #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-743339903
Answering the question from the other thread - after fixing the static check it should be fine to merge (as long as you did some manual tests with snowflake :). We do not (yet) have system tests to automatically verify such operators better than in unit tests and mocking.
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-817220913
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] github-actions[bot] commented on pull request #13017: optional schema configuration of S3ToSnowflakeTransferOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13017:
URL: https://github.com/apache/airflow/pull/13017#issuecomment-748592203
[The Workflow run](https://github.com/apache/airflow/actions/runs/433781268) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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