aminghadersohi commented on PR #44695: URL: https://github.com/apache/superset/pull/44695#issuecomment-5902973032
AI review at `45309e2e`: no blocking findings, no new commits. **Changes since the approval at `d375346e`** - `4954f82e`: validation always uses `subject=None`. I checked this against shillelagh 1.4.5 and SQLAlchemy 2.0.52. `APSWGSheetsDialect.create_connect_args` does put the URL `subject` into `adapter_kwargs`. But `update_params_from_encrypted_extra` supplies its own `connect_args["adapter_kwargs"]`, and SQLAlchemy's merge is shallow, so that key replaces the dialect's and the subject is dropped. Service-account queries therefore never send a subject. Validation now uses the same identity as queries. It no longer lets an admin add a sheet that only their delegated identity can read and the service account can't query. There is no path where a user queries as a subject they weren't validated for. Empty-credential and OAuth connections behave as before. `test_query_service_account_subject` checks the real DBAPI `connect` arguments. - `ddfec16e`: timezone-aware datetimes are converted to naive UTC, which is documented. Nullable `Int64` values stay ints and nulls become `""`. `time` values become ISO strings. The input dataframe isn't modified. The approver's non-blocking notes on timezones, `Int64`, `time` coverage and reuse of the `data` variable are all addressed. - `45309e2e`: test-only. **Earlier review threads:** both of EnxDev's inline threads (the shallow-merge subject and the modal's forced `impersonate_user`) are resolved by `4954f82e`, and the tests cover create, edit and all three flag states. All other bot threads are resolved. **Checks:** `tests/unit_tests/db_engine_specs/test_gsheets.py` 78 passed. Pre-commit on the three changed files passed, including mypy, ruff and pylint. -- 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]
