SameerMesiah97 commented on code in PR #62043:
URL: https://github.com/apache/airflow/pull/62043#discussion_r2814328515
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -341,6 +345,14 @@ def _get_static_conn_params(self) -> dict[str, str | None]:
time-sensitive values such as OAuth access tokens. This is used in
``_get_valid_oauth_token()`` and ``get_conn_params()``.
"""
+ static_token = self._get_field(extra_dict, "token")
+ refresh_token = self._get_field(extra_dict, "refresh_token")
+
+if static_token and refresh_token:
+ raise AirflowException(
+ "Cannot use both 'token' and 'refresh_token' in Snowflake connection."
+ )
Review Comment:
The indentation of lines 351-354 is not correct, rendering it syntactically
invalid. And even then it looks like you are just getting `static_token` from
`extra_dict` but not setting it in `conn_config` in the 'token' field (which is
where I would expect a static token to be persisted)
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -314,6 +314,10 @@ def _get_conn_params(self) -> dict[str, str | None]:
conn_config = dict(static_config)
if conn_config.get("authenticator") == "oauth":
+ # Static token case
+ if conn_config.get("token"):
+ pass
Review Comment:
Isn't this condition inert? It seems like it is doing nothing at all. If you
are just using a PAT-like access token (which I am assuming are not meant to be
refreshed), you should not trigger the access token retrieval flow (please see
`_get_valid_oauth_token`), which will not check for refresh token before being
invoked (unless you don't pass a `grant_type` which will raise an error).
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -870,3 +882,27 @@ def get_openlineage_database_specific_lineage(self,
task_instance) -> OperatorLi
)
return None
+ class TestSnowflakeHook:
+
Review Comment:
Why is this test here? The existing test suite for `SnowflakeHook` is
located in:
`providers\snowflake\tests\unit\snowflake\hooks\test_snowflake.py`
And again, there are identation errors that render the code for this test
syntactically invalid. The test does not follow existing conventions in the
test suite. I would look at it and try to follow the patterns in there.
--
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]
