villebro commented on code in PR #30081:
URL: https://github.com/apache/superset/pull/30081#discussion_r1802012751
##########
superset/db_engine_specs/base.py:
##########
@@ -552,18 +556,16 @@ def get_oauth2_token(
"""
timeout = current_app.config["DATABASE_OAUTH2_TIMEOUT"].total_seconds()
uri = config["token_request_uri"]
- response = requests.post(
- uri,
- json={
- "code": code,
- "client_id": config["id"],
- "client_secret": config["secret"],
- "redirect_uri": config["redirect_uri"],
- "grant_type": "authorization_code",
- },
- timeout=timeout,
- )
- return response.json()
+ req_body = {
+ "code": code,
+ "client_id": config["id"],
+ "client_secret": config["secret"],
+ "redirect_uri": config["redirect_uri"],
+ "grant_type": "authorization_code",
+ }
+ if config["request_content_type"] == "data":
+ return requests.post(uri, data=req_body, timeout=timeout).json()
+ return requests.post(uri, json=req_body, timeout=timeout).json()
Review Comment:
I agree that data is likely the more common implementation. As we're still
in early days, I feel it's well motivated to introduce a breaking change, as
long as we have an `UPDATING.md` explaining the required steps to get it to
work again. This in the interest of having a clean/idiomatic API in the long
term..
##########
superset/db_engine_specs/base.py:
##########
@@ -430,9 +430,10 @@ class BaseEngineSpec: # pylint:
disable=too-many-public-methods
# Does the engine supports OAuth 2.0? This requires logic to be added to
one of the
# the user impersonation methods to handle personal tokens.
supports_oauth2 = False
- oauth2_scope = ""
- oauth2_authorization_request_uri: str | None = None # pylint:
disable=invalid-name
- oauth2_token_request_uri: str | None = None
+ oauth2_scope: str = ""
+ oauth2_authorization_request_uri: str | None = "" # pylint:
disable=invalid-name
+ oauth2_token_request_uri: str | None = ""
+ oauth2_token_request_type: str | None = ""
Review Comment:
nit: If we're defaulting to `""`, when might these still be `None`?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
