sha174n commented on PR #43020: URL: https://github.com/apache/superset/pull/43020#issuecomment-5594357706
@rusackas Follow-up in b2ed809, two things a review pass turned up: - The unit tests all mocked `validator.validate` to return `[]`, so the `to_dict()` serialization on the result path lost its only assertion when `test_validate_sql_errors` went away. Added a unit test that asserts a non-empty annotation list is serialized into the command result, so that contract is covered without needing a backend. - `docs/src/data/databases.json` had stale scores: `diagnose()` weights `sql_validation` at 10 points, so PostgreSQL, Aurora PostgreSQL and Aurora PostgreSQL (Data API) should be 94, not 104. `averageScore` is unchanged at 53. The four remaining `validate_sql` integration tests are still presto-gated and no CI job runs that file with a Presto example DB, so they skip everywhere. Happy to either mark them so the presto job picks them up or drop them, whichever you prefer, in this PR or 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]
