sha174n commented on PR #43020: URL: https://github.com/apache/superset/pull/43020#issuecomment-5593593754
@rusackas All addressed in 6b42952: - `UPDATING.md` breaking-change note, covering the behaviour change, the `SQL_VALIDATORS_BY_ENGINE` config change (including the error a stale `"postgresql"` entry produces) and the dependency removal. - `pgsanity` dropped from `pyproject.toml` (and the `isort` `known_third_party` list) plus both requirements lockfiles. - `libecpg-dev` dropped from the `Dockerfile` and also from `setup-postgres` in `.github/workflows/bashlib.sh`, which was installing it on every Postgres CI job. That also fixes a pre-existing unbalanced `::group::` there. - Both bito nits: the backend guards are now consistently `!= "presto"`, and the `mock_database` docstring matches the fixture. - PR template filled out in the description. Two things I found while in there and folded in: the docs generator and the checked-in `databases.json` still advertised `sql_validation` for PostgreSQL/Aurora PostgreSQL, and `db_engine_specs/README.md` still said "Presto and Postgres". One thing worth your call, noted at the bottom of the description: those `validate_sql` integration tests were only ever exercised by the Postgres CI backend, so gating them on Presto leaves the endpoint without integration coverage. I removed `test_validate_sql_errors` since it asserted the `pgsanity` error string, and can re-point the rest at a stubbed validator here or in a follow-up. -- 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]
