bito-code-review[bot] commented on code in PR #40071:
URL: https://github.com/apache/superset/pull/40071#discussion_r3227878648
##########
superset/utils/oauth2.py:
##########
@@ -129,15 +129,26 @@ def refresh_oauth2_token(
database_id: int,
user_id: int,
db_engine_spec: type[BaseEngineSpec],
- token: DatabaseUserOAuth2Tokens,
) -> str | None:
+ # pylint: disable=import-outside-toplevel
+ from superset.models.core import DatabaseUserOAuth2Tokens
+
# Use longer TTL for OAuth2 token refresh (may involve network calls)
with DistributedLock(
namespace="refresh_oauth2_token",
ttl_seconds=30,
user_id=user_id,
database_id=database_id,
):
+ # Short circuit in case another request already deleted the token
+ token = (
+ db.session.query(DatabaseUserOAuth2Tokens)
+ .filter_by(user_id=user_id, database_id=database_id)
+ .one_or_none()
+ )
+ if token is None:
+ return None
+
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing concurrency check in token refresh</b></div>
<div id="fix">
The refresh_oauth2_token function now fetches the token inside the lock but
lacks a check for whether the access token is already valid (not expired). In
concurrent requests, this causes redundant refresh calls even if another thread
already refreshed the token, leading to performance issues and potential
failures with single-use refresh tokens. Add the validity check to
short-circuit and return early if the token is still fresh.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
return None
if token.access_token and datetime.now() <
token.access_token_expiration:
return token.access_token
````
</div>
</details>
</div>
<small><i>Code Review Run #46ddfb</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
