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

[GitHub] [airflow] potiuk commented on pull request #13152: Convert columns in get_pandas_df to lowercase SnowflakeHook

potiuk commented on pull request #13152:
URL: https://github.com/apache/airflow/pull/13152#issuecomment-785324499


   Yeah. The new version of python connector is out and we switched to it. But after looking closely - I also think we should not merge this one - similarly to Daniel, this is  -0 for me. Since everything we do is python, overriding the get_pandas_df in your own operator is almost as easy as passing a parameter and it gives you much better flexibility. 
   
   Also, I think if Snowflake has a captitalised names, the best you can do is to keep it this way throughout the whole "data" journey - to avoid confusion, even for the cases where you have to do any kind of matching/lineage kind of analysis, it simply makes sense to keep it consistent. Adding an "optional parameter" seems like it is not dangerous, but I think we should avoid doing so if the option is "niche" and when you can do it almost as easily by other means.
   
   I think we are all for closing the PR. Are you convinced @JavierLopezT ?1


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