ktmud opened a new pull request, #20761: URL: https://github.com/apache/superset/pull/20761
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY DB migration introduced by #20359 did not run through in Airbnb environment and throws this error in our MySQL database: ``` INFO [alembic.runtime.migration] Running upgrade c747c78868b6 -> 06e1e70058c7, Migrating legacy Area Upgrading (1/21668): another#127 Traceback (most recent call last): File "/usr/local/lib/python3.9/dist-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context self.dialect.do_execute( File "/usr/local/lib/python3.9/dist-packages/sqlalchemy/engine/default.py", line 732, in do_execute cursor.execute(statement, parameters) File "/usr/local/lib/python3.9/dist-packages/MySQLdb/cursors.py", line 183, in execute while self.nextset(): File "/usr/local/lib/python3.9/dist-packages/MySQLdb/cursors.py", line 137, in nextset nr = db.next_result() MySQLdb._exceptions.ProgrammingError: (2014, "Commands out of sync; you can't run this command now") ``` This is because updating objects within `iter_per` does not work well with MySQL cursors. (Maybe other databases will have similar issues if anyone with more than 1000 area charts can test?) The solution is to migrate to [paginated_update](https://github.com/apache/superset/blob/cadd259788c99415862cef7e8a5da9aaf4ed12cd/superset/migrations/shared/utils.py#L94) in `superset.migrations.shared.utils`, which runs manual pagination with `OFFSET` and `LIMIT` instead of streaming results with DBAPI cursors. While optimizing this, I also relocated to the migration files (and the corresponding tests) from superset root to `superset.migrations` as migration code should be as self-contained and stable as possible, since app code are updated much more frequently. While testing, I also noticed that some charts will fail at reloading `query_context` as the combined JSON payload is too large and the default Text column in MySQL was not able to save the full string. We need to migrate both `Slice.query_context` and `Slice.params` to `MediumText`, which I will address in another PR. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A ### TESTING INSTRUCTIONS CI and tested locally with Airbnb db instances. The area chart migration was finished in ~20 seconds for about ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [x] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [x] Migration is atomic, supports rollback & is backwards-compatible - [x] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
