Fix PostgreSQL compatibility issues in CI and SQL profiling#2186
Fix PostgreSQL compatibility issues in CI and SQL profiling#2186abdibaker wants to merge 1 commit intodjango-commons:mainfrom
Conversation
Fixes django-commons#2185 - Add vendor detection to SQL panel profiling to prevent MySQL-specific commands on PostgreSQL - Fix CI PostgreSQL job environment variables - Fix malformed lint job strategy configuration - Add comprehensive PostgreSQL compatibility tests Changes: - debug_toolbar/panels/sql/forms.py: Add vendor check in profile() method - .github/workflows/test.yml: Add DB_* env vars for PostgreSQL, fix lint job - tests/test_postgresql_compatibility.py: New test file for PostgreSQL fixes - docs/changes.rst: Document PostgreSQL compatibility improvements
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
I think the idea of this change is a quality one, but I think we should move the logic up into the form layer. Try subclassing SQLSelectForm with some custom validation to do this check. That will protect this in the sql_profile and keep our instrumentation logic cleaner.
We should also prevent the profile button from appearing when using an incompatible vendor.
Lastly, we may want to check Django's own backend features to see if this is something we can base off Django's own checks. There's no sense in both repositories maintaining a boolean constant about whether or not this feature is supported in MySQL.
| if connection.vendor == "mysql": | ||
| self.skipTest("This test is for non-MySQL databases") |
There was a problem hiding this comment.
This should be moved up as a decorator. Search for other usages of connection.vendor and tests that are skipped.
2 participants
Description
This PR fixes PostgreSQL compatibility issues that were causing CI pipeline failures. The main issue was that the SQL panel's profiling feature was attempting to use MySQL-specific commands on PostgreSQL databases, resulting in errors like "unrecognized configuration parameter profiling".
Additionally, the CI configuration had missing environment variables for PostgreSQL test jobs and a malformed lint job configuration.
Key Changes:
Root Cause: The
SQLSelectForm.profile()method was executing MySQL-specificSET PROFILINGcommands and queryinginformation_schema.profilingwithout checking the database vendor, causing PostgreSQL CI jobs to fail.Solution: Added vendor detection that raises a clear
ValueErrorfor non-MySQL databases, preventing the MySQL-specific code from executing on PostgreSQL/SQLite.Fixes #2185
Checklist:
docs/changes.rst.