sunank200 commented on code in PR #47191:
URL: https://github.com/apache/airflow/pull/47191#discussion_r1975031486
##########
providers/snowflake/docs/connections/snowflake.rst:
##########
@@ -39,10 +39,11 @@ Configuring the Connection
--------------------------
Login
- Specify the snowflake username.
+ Specify the snowflake username. For OAuth, the OAuth Client ID.
Review Comment:
Should we also create a separate section for OAuth?
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -185,6 +187,30 @@ def _get_field(self, extra_dict, field_name):
return extra_dict[field_name] or None
return extra_dict.get(backcompat_key) or None
+ def get_oauth_token(self, conn_config: dict) -> str:
Review Comment:
Do we need to retry for the request?
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -185,6 +187,30 @@ def _get_field(self, extra_dict, field_name):
return extra_dict[field_name] or None
return extra_dict.get(backcompat_key) or None
+ def get_oauth_token(self, conn_config: dict) -> str:
+ """Generate temporary OAuth access token using refresh token in
connection details."""
+ url =
f"https://{conn_config['account']}.snowflakecomputing.com/oauth/token-request"
+ data = {
+ "grant_type": "refresh_token",
+ "refresh_token": conn_config["refresh_token"],
+ "redirect_uri": conn_config.get("redirect_uri",
"https://localhost.com";),
+ }
+ response = requests.post(
+ url,
+ data=data,
+ headers={
+ "Content-Type": "application/x-www-form-urlencoded",
+ },
+ auth=HTTPBasicAuth(conn_config["client_id"],
conn_config["client_secret"]), # type: ignore[arg-type]
Review Comment:
Is `client_id` and `client_id` always defined? Should there be a check
before getting OAuth token?
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -185,6 +187,30 @@ def _get_field(self, extra_dict, field_name):
return extra_dict[field_name] or None
return extra_dict.get(backcompat_key) or None
+ def get_oauth_token(self, conn_config: dict) -> str:
+ """Generate temporary OAuth access token using refresh token in
connection details."""
+ url =
f"https://{conn_config['account']}.snowflakecomputing.com/oauth/token-request"
+ data = {
+ "grant_type": "refresh_token",
+ "refresh_token": conn_config["refresh_token"],
Review Comment:
what if `refresh_token` is not there? `conn_config["refresh_token"]` would
throw an error.
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -185,6 +187,30 @@ def _get_field(self, extra_dict, field_name):
return extra_dict[field_name] or None
return extra_dict.get(backcompat_key) or None
+ def get_oauth_token(self, conn_config: dict) -> str:
+ """Generate temporary OAuth access token using refresh token in
connection details."""
+ url =
f"https://{conn_config['account']}.snowflakecomputing.com/oauth/token-request"
+ data = {
+ "grant_type": "refresh_token",
+ "refresh_token": conn_config["refresh_token"],
+ "redirect_uri": conn_config.get("redirect_uri",
"https://localhost.com";),
Review Comment:
why default to localhost?
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -185,6 +187,30 @@ def _get_field(self, extra_dict, field_name):
return extra_dict[field_name] or None
return extra_dict.get(backcompat_key) or None
+ def get_oauth_token(self, conn_config: dict) -> str:
+ """Generate temporary OAuth access token using refresh token in
connection details."""
+ url =
f"https://{conn_config['account']}.snowflakecomputing.com/oauth/token-request"
+ data = {
+ "grant_type": "refresh_token",
+ "refresh_token": conn_config["refresh_token"],
+ "redirect_uri": conn_config.get("redirect_uri",
"https://localhost.com";),
+ }
+ response = requests.post(
+ url,
+ data=data,
+ headers={
+ "Content-Type": "application/x-www-form-urlencoded",
+ },
+ auth=HTTPBasicAuth(conn_config["client_id"],
conn_config["client_secret"]), # type: ignore[arg-type]
+ )
+
+ try:
+ response.raise_for_status()
Review Comment:
Do we need to try except and raise here? We already have
`response.raise_for_status()`
--
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]
