aminghadersohi commented on PR #44709: URL: https://github.com/apache/superset/pull/44709#issuecomment-5884253582
Addressing the Additional Suggestions in Code Review Agent Run #62eb45 (`tests/unit_tests/db_engine_specs/test_gsheets.py`): - **Inline imports (rule 12745)**: fixed in 9b141eb762. `create_engine` now comes from a module-level `import sqlalchemy`. A plain module-level `create_engine` would be shadowed by the local `create_engine` mocks in the existing `get_table_names` tests. The local `GSheetsEngineSpec` imports stay, and a comment in the module header documents why: `superset/db_engine_specs/gsheets.py` binds `superset.db` and `superset.security_manager` at import time, so it has to load after the app fixture has initialized them. - **Unannotated mock variables (rule 12787)**: fixed in 9b141eb762. The `user` and `database` mocks added by this PR are annotated as `MagicMock`. The other findings were already covered: the subject on the tokenless service-account path in f6083492f8, the subject with a token in 263c191705, and the redundant `SupersetException` import in fa1309e1a0. All 41 GSheets unit tests and the file-scoped pre-commit checks pass. -- 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]
