You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "joeknize-bc (via GitHub)" <gi...@apache.org> on 2023/03/10 01:05:12 UTC
[GitHub] [airflow] joeknize-bc opened a new issue, #30010: SnowflakeOperator default autocommit flipped to False
joeknize-bc opened a new issue, #30010:
URL: https://github.com/apache/airflow/issues/30010
### Apache Airflow Provider(s)
snowflake
### Versions of Apache Airflow Providers
This started with apache-airflow-providers-snowflake==4.0.0 and is still an issue with 4.0.4
### Apache Airflow version
2.5.1
### Operating System
Debian GNU/Linux 11 (bullseye)
### Deployment
Astronomer
### Deployment details
This is affecting both local and hosted deployments
### What happened
We are testing out several updated packages, and one thing that broke was the SnowflakeOperator when it was executing a stored procedure. The specific error points to autocommit being set to False:
`Stored procedure execution error: Scoped transaction started in stored procedure is incomplete and it was rolled back.`
Whereas this used to work in version 3.2.0:
```
copy_data_snowflake = SnowflakeOperator(
task_id=f'copy_{table_name}_snowflake',
sql=query,
)
```
In order for it to work now, we have to specify autocommit=True:
```
copy_data_snowflake = SnowflakeOperator(
task_id=f'copy_{table_name}_snowflake',
sql=query,
autocommit=True,
)
```
[The code](https://github.com/apache/airflow/blob/599c587e26d5e0b8fa0a0967f3dc4fa92d257ed0/airflow/providers/snowflake/operators/snowflake.py#L45) still indicates that the default is True, but I believe [this commit](https://github.com/apache/airflow/commit/ecd4d6654ff8e0da4a7b8f29fd23c37c9c219076#diff-e9f45fcabfaa0f3ed0c604e3bf2215fed1c9d3746e9c684b89717f9cd75f1754L98) broke it.
### What you think should happen instead
The default for autocommit should revert to the previous behavior, matching the documentation.
### How to reproduce
In Snowflake:
```
CREATE OR REPLACE TABLE PUBLIC.FOO (BAR VARCHAR);
CREATE OR REPLACE PROCEDURE PUBLIC.FOO()
RETURNS VARCHAR
LANGUAGE SQL
AS $$
INSERT INTO PUBLIC.FOO VALUES('bar');
$$
;
```
In Airflow, this fails:
```
copy_data_snowflake = SnowflakeOperator(
task_id='call_foo',
sql="call public.foo()",
)
```
But this succeeds:
```
copy_data_snowflake = SnowflakeOperator(
task_id='call_foo',
sql="call public.foo()",
autocommit=True,
)
```
### Anything else
It looks like this may be an issue with stored procedures specifically. If I instead do this:
```
copy_data_snowflake = SnowflakeOperator(
task_id='call_foo',
sql="INSERT INTO PUBLIC.FOO VALUES('bar');",
)
```
The logs show that although autocommit is confusingly set to False, a `COMMIT` statement is executed:
```
[2023-03-09, 18:43:09 CST] {cursor.py:727} INFO - query: [ALTER SESSION SET autocommit=False]
[2023-03-09, 18:43:09 CST] {cursor.py:740} INFO - query execution done
[2023-03-09, 18:43:09 CST] {cursor.py:878} INFO - Number of results in first chunk: 1
[2023-03-09, 18:43:09 CST] {sql.py:375} INFO - Running statement: INSERT INTO PUBLIC.FOO VALUES('bar');, parameters: None
[2023-03-09, 18:43:09 CST] {cursor.py:727} INFO - query: [INSERT INTO PUBLIC.FOO VALUES('bar');]
[2023-03-09, 18:43:09 CST] {cursor.py:740} INFO - query execution done
[2023-03-09, 18:43:09 CST] {sql.py:384} INFO - Rows affected: 1
[2023-03-09, 18:43:09 CST] {snowflake.py:380} INFO - Rows affected: 1
[2023-03-09, 18:43:09 CST] {snowflake.py:381} INFO - Snowflake query id: 01aad76b-0606-feb5-0000-26b511d0ba02
[2023-03-09, 18:43:09 CST] {cursor.py:727} INFO - query: [COMMIT]
```
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1464467636
> @eladkal I just triple-checked the logs for the SnowflakeOperator tasks in my DAGs, no deprecation warnings present. Am I not looking in the right place?
https://github.com/apache/airflow/blob/b9c231ceb0f3053a27744b80e95f08ac0684fe38/airflow/providers/snowflake/operators/snowflake.py#L94-L102
--
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
[GitHub] [airflow] joeknize-bc commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "joeknize-bc (via GitHub)" <gi...@apache.org>.
joeknize-bc commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1470039159
Will do - if I continue to not see it I'll open a new issue for that. Thanks for your help!
--
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
[GitHub] [airflow] joeknize-bc commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "joeknize-bc (via GitHub)" <gi...@apache.org>.
joeknize-bc commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1464447139
@eladkal I just triple-checked the logs for the SnowflakeOperator tasks in my DAGs, no deprecation warnings present. Am I not looking in the right place?
--
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
[GitHub] [airflow] potiuk commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "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
[GitHub] [airflow] eladkal commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1464148485
@joeknize-bc Indications are in the code. Check your logs it raises deprecation warnings.
--
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
[GitHub] [airflow] potiuk commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1468912997
Maybe you have deprecation warnings disabled. They are enabled by default:
```
if not sys.warnoptions:
warnings.filterwarnings(action="default", category=DeprecationWarning, module="airflow")
warnings.filterwarnings(action="default", category=PendingDeprecationWarning, module="airflow")
```
Maybe you can check Astronoker documentation of Astronomer deployment and reach out to Astronomer support if you should see them, since you are on a managed version.
--
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
[GitHub] [airflow] eladkal commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1463550105
Ill check it but either way the operator is deprecated and will be removed in future release. You should switch to SQLExecuteQueryOperator
--
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
[GitHub] [airflow] eladkal commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1463665654
I think we should leave it as is.
remove the wrong doc and add clarification to the release notes of the relevant snowflake version that there was intentional behavior 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #30010: SnowflakeOperator default autocommit flipped to False
URL: https://github.com/apache/airflow/issues/30010
--
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
[GitHub] [airflow] joeknize-bc commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "joeknize-bc (via GitHub)" <gi...@apache.org>.
joeknize-bc commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1464129741
Ok, I understand the situation and agree with the least-bad option of updating the documentation.
I was surprised to read that it is deprecated - there's nothing on [this page](https://airflow.apache.org/docs/apache-airflow-providers-snowflake/stable/operators/snowflake.html) that indicates it, and there's no deprecation warning showing up in my logs.
--
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
[GitHub] [airflow] boring-cyborg[bot] commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1463055322
Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
--
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
[GitHub] [airflow] potiuk commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1463677177
Added PR with doc 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] joeknize-bc commented on issue #30010: SnowflakeOperator default autocommit flipped to False
Posted by "joeknize-bc (via GitHub)" <gi...@apache.org>.
joeknize-bc commented on issue #30010:
URL: https://github.com/apache/airflow/issues/30010#issuecomment-1464568811
I see the code, but not the message in my logs. Please confirm which log should I expect to find that message in - it's not in my task log.
--
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