[GitHub] [airflow] potiuk commented on issue #30010: SnowflakeOperator default autocommit flipped to False

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1463629032

   I think that was indeed a behaviour change. But I am not sure if we should change it to be default or rather update the documentation to reflect the current state. 
   
   I am a bit torn on that one, but since we already released and have "in-the-wild" versions out there with autocommit = False, I woudl be fore just updating the docs and keeping the breaking change.
   
   Snowflake is the only SQL operator that after the unification would have "autocommit" = True. And the unification of the interface was an important aspect of the change and there were other smaller or bigger. As explained in https://airflow.apache.org/docs/apache-airflow-providers-snowflake/stable/index.html#id4 , 4.0.* line (4.0.1 and 4.0.2 were yanked as they did not really work) we unified all DB operators to conform to the same semantics as other operators. 
   So having a breaking change in this release like that was quite possible. The operator is indeed deprecated, so you should switch to SQLExecuteQueryOperator instead (and there autocommit is and will be False by default).
   
   It is super-easy to fix it - by adding autocommit=True, and we could make sure to mention it it in the release notes in our documentation. 
   
   Probably if we realized it then, leaving the "autocomit = True" woudl be a better option for compatibility, but now - the jinni is out of the bottle. In order to actually fix it now we would literally have to bump the version to 5.0* and make it a breaking change for anyone who already started using 4.0.3+. So we don't have a "good" solution here - either we keep the change that breaks 3.* users or we introduce a new breaking change for those who started to use 4.*. Taking into account that we already deprecated it and suggest to use the generic SQL operator with autocommit = False, I think it woudl be much more confusing if go back to True.
   
   As bad as it is, I think we should swallow the bitter pill and simply update the docs.
   
   WDYT?
   
   
   


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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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