codeant-ai-for-open-source[bot] commented on code in PR #38443:
URL: https://github.com/apache/superset/pull/38443#discussion_r2941899475
##########
superset/db_engine_specs/gsheets.py:
##########
@@ -389,8 +404,14 @@ def validate_parameters(
engine = create_engine(
"gsheets://",
- service_account_info=encrypted_credentials,
- subject=subject,
+ connect_args={
+ "adapter_kwargs": {
+ "gsheetsapi": {
+ "service_account_info": encrypted_credentials,
+ "subject": subject,
+ }
+ }
+ },
)
Review Comment:
**Suggestion:** The new engine configuration always sends
`service_account_info`, even when it is just the default empty `{}` (common for
OAuth2-only connections). Passing an empty service-account payload through
`adapter_kwargs.gsheetsapi` can trigger credential-construction errors during
`engine.connect()` before the OAuth2 short-circuit logic runs. Only include
`service_account_info` when real credentials are present. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ GSheets OAuth2 validation can fail during database setup.
- ❌ `/validate_parameters/` may error before OAuth2 skip logic.
- ⚠️ Admins blocked from saving OAuth2-only GSheets connections.
```
</details>
```suggestion
gsheetsapi_kwargs: dict[str, Any] = {"subject": subject}
if encrypted_credentials:
gsheetsapi_kwargs["service_account_info"] = encrypted_credentials
engine = create_engine(
"gsheets://",
connect_args={
"adapter_kwargs": {
"gsheetsapi": gsheetsapi_kwargs,
}
},
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call `POST /api/v1/database/validate_parameters/` (endpoint defined at
`superset/databases/api.py:1952-1961`) with engine `gsheets`, OAuth2 config
present, and
no real `service_account_info` (common payload shape validated in tests at
`tests/unit_tests/db_engine_specs/test_gsheets.py:964-993`).
2. Request executes `ValidateDatabaseParametersCommand.run()`
(`superset/commands/database/validate.py:45`) which immediately calls
`engine_spec.validate_parameters(self._properties)` (`validate.py:69`) for
GSheets.
3. In `GSheetsEngineSpec.validate_parameters`
(`superset/db_engine_specs/gsheets.py:392-416`), missing/empty
`service_account_info` is
coerced to `"{}"` then parsed to `{}`, and `create_engine(...
gsheetsapi.service_account_info={})` is always used before
`engine.connect()`.
4. `engine.connect()` runs before OAuth2 short-circuit logic
(`is_oauth2_conn` computed
later at `gsheets.py:425`), so credential construction can fail first; this
is consistent
with handled error text in `needs_oauth2` (`gsheets.py:216-217`), causing
validation to
fail before OAuth2 skip behavior applies.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/gsheets.py
**Line:** 405:415
**Comment:**
*Logic Error: The new engine configuration always sends
`service_account_info`, even when it is just the default empty `{}` (common for
OAuth2-only connections). Passing an empty service-account payload through
`adapter_kwargs.gsheetsapi` can trigger credential-construction errors during
`engine.connect()` before the OAuth2 short-circuit logic runs. Only include
`service_account_info` when real credentials are present.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38443&comment_hash=46599aa7b473b1a6632abccbfbae67878477281095ba3e5567808a13452a846b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38443&comment_hash=46599aa7b473b1a6632abccbfbae67878477281095ba3e5567808a13452a846b&reaction=dislike'>👎</a>
--
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]
