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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to