Fix column type on dbs.encrypted_extra, add instructions for testing migration downgrades

[willbarrett]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

The new encrypted_extra field on the dbs table was causing the databases list to become inaccessible when used with Postgres. This was caused by a type misalignment between the database and the model definition. Also add instructions in CONTRIBUTING.md around testing down migrations.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

• Set up a BigQuery connection according to instructions here: Provide documentation for using a Service Account to connect to BigQuery #8462
• Ensure the databases/list page is still accessible

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

@dpgaspar @mistercrunch

[codecov-io]

Codecov Report

Merging #8493 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8493   +/-   ##
=======================================
  Coverage   66.61%   66.61%           
=======================================
  Files         449      449           
  Lines       22610    22610           
  Branches     2367     2367           
=======================================
  Hits        15061    15061           
  Misses       7411     7411           
  Partials      138      138

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 554a6d8...95dee01. Read the comment docs.

[elukey]

Hi everybody, for some the first time that I migrated from 0.35.2 I didn't see any bug, but then I had to re-do it and I noticed the following when running db upgrade:

[..]
  File "/srv/deployment/analytics/superset/venv/lib/python3.7/site-packages/sqlalchemy/dialects/mysql/base.py", line 2035, in visit_VARCHAR
    "VARCHAR requires a length on dialect %s" % self.dialect.name
sqlalchemy.exc.CompileError: VARCHAR requires a length on dialect mysql

I found something similar in #9878 but not really sure where this comes from. I am running Mariadb 10.3.22, is it something db-specific? Has anybody else already encountered this problem?

If I check the specific line in base.py (belongs to MySQLTypeCompiler) I see the following:

    def visit_VARCHAR(self, type_, **kw):
        if type_.length:
            return self._extend_string(type_, {}, "VARCHAR(%d)" % type_.length)
        else:
            raise exc.CompileError(
                "VARCHAR requires a length on dialect %s" % self.dialect.name
            )

EncryptedType doesn't seem to have a length afaics, maybe there is a quick fix to support Mariadb/Mysql as well?

[villebro]

This is what broke it: kvesteri/sqlalchemy-utils#426

The pinned version on master branch works. I suggest downgrading to that until this issue is resolved in sqlalchemy-utils.

[elukey]

@villebro I confirm that pinning sqlalchemy-utils==0.36.4 solves the problem, thanks!

[villebro]

I'll start working on 0.36.1 so we can get this fix out to the broader community.