From ea0eedcd66b79ee9969a3c5eb64f276e24cb55db Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Fri, 1 Nov 2019 09:52:01 -0700 Subject: [PATCH 1/5] Fix column type on dbs.encrypted_extra --- ...f3df2_alter_type_of_dbs_encrypted_extra.py | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py diff --git a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py new file mode 100644 index 000000000000..af887c6923a6 --- /dev/null +++ b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py @@ -0,0 +1,55 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""alter type of dbs encrypted_extra + + +Revision ID: c2acd2cf3df2 +Revises: cca2f5d568c8 +Create Date: 2019-11-01 09:18:36.953603 + +""" + +from alembic import op + +import sqlalchemy as sa + +from sqlalchemy_utils import EncryptedType + +# revision identifiers, used by Alembic. +revision = "c2acd2cf3df2" +down_revision = "cca2f5d568c8" + + +def upgrade(): + op.alter_column( + "dbs", + "encrypted_extra", + existing_type=sa.Text(), + type_=EncryptedType(sa.Text()), + postgresql_using="encrypted_extra::bytea", + existing_nullable=True, + ) + + +def downgrade(): + op.alter_column( + "dbs", + "encrypted_extra", + existing_type=EncryptedType(sa.Text()), + type_=sa.Text(), + existing_nullable=True, + ) From df0789e716c234c8dbc0c807d42fcea479a1ac36 Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Fri, 1 Nov 2019 10:16:26 -0700 Subject: [PATCH 2/5] Add instructions for testing migration downgrades --- CONTRIBUTING.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a0801792aa48..109a080429eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -768,6 +768,20 @@ https://github.com/apache/incubator-superset/pull/3013 [Example commit](https://github.com/apache/incubator-superset/pull/5745/commits/6220966e2a0a0cf3e6d87925491f8920fe8a3458) +1. Test the migration's `down` method + + ```bash + superset db downgrade + ``` + + The output should look like this: + + ``` + INFO [alembic.runtime.migration] Context impl SQLiteImpl. + INFO [alembic.runtime.migration] Will assume transactional DDL. + INFO [alembic.runtime.migration] Running downgrade 40a0a483dd12 -> 1a1d627ebd8e, add_metadata_column_to_annotation_model.py + ``` + ### Merging DB migrations When two DB migrations collide, you'll get an error message like this one: From 8f7eb6ec5e32cc1ffcaf20095aa281b0a8ace7eb Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Fri, 1 Nov 2019 11:07:30 -0700 Subject: [PATCH 3/5] Account for non-Postgres DBs in migration --- ...f3df2_alter_type_of_dbs_encrypted_extra.py | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py index af887c6923a6..1f23040ae6ed 100644 --- a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py +++ b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py @@ -35,14 +35,25 @@ def upgrade(): - op.alter_column( - "dbs", - "encrypted_extra", - existing_type=sa.Text(), - type_=EncryptedType(sa.Text()), - postgresql_using="encrypted_extra::bytea", - existing_nullable=True, - ) + try: + # Postgres migration + op.alter_column( + "dbs", + "encrypted_extra", + existing_type=sa.Text(), + type_=EncryptedType(sa.Text()), + postgresql_using="encrypted_extra::bytea", + existing_nullable=True, + ) + except TypeError: + # non-Postgres migration + op.alter_column( + "dbs", + "encrypted_extra", + existing_type=sa.Text(), + type_=EncryptedType(sa.Text()), + existing_nullable=True, + ) def downgrade(): From 07ef9b94170a1e52aa2ab50955346871c6363dbb Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Fri, 1 Nov 2019 11:50:38 -0700 Subject: [PATCH 4/5] Use batch_alter_table to make SQLite happy --- ...f3df2_alter_type_of_dbs_encrypted_extra.py | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py index 1f23040ae6ed..ea247ee2fa21 100644 --- a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py +++ b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py @@ -35,32 +35,32 @@ def upgrade(): - try: - # Postgres migration - op.alter_column( - "dbs", - "encrypted_extra", - existing_type=sa.Text(), - type_=EncryptedType(sa.Text()), - postgresql_using="encrypted_extra::bytea", - existing_nullable=True, - ) - except TypeError: - # non-Postgres migration - op.alter_column( - "dbs", - "encrypted_extra", - existing_type=sa.Text(), - type_=EncryptedType(sa.Text()), - existing_nullable=True, - ) + with op.batch_alter_table("dbs") as batch_op: + try: + # Postgres migration + batch_op.alter_column( + "encrypted_extra", + existing_type=sa.Text(), + type_=EncryptedType(sa.Text()), + postgresql_using="encrypted_extra::bytea", + existing_nullable=True, + ) + except TypeError: + # non-Postgres migration + batch_op.alter_column( + "dbs", + "encrypted_extra", + existing_type=sa.Text(), + type_=EncryptedType(sa.Text()), + existing_nullable=True, + ) def downgrade(): - op.alter_column( - "dbs", - "encrypted_extra", - existing_type=EncryptedType(sa.Text()), - type_=sa.Text(), - existing_nullable=True, - ) + with op.batch_alter_table("dbs") as batch_op: + batch_op.alter_column( + "encrypted_extra", + existing_type=EncryptedType(sa.Text()), + type_=sa.Text(), + existing_nullable=True, + ) From 95dee01ec2e536aec32496c1732d2d380a94640b Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Fri, 1 Nov 2019 12:41:25 -0700 Subject: [PATCH 5/5] Another CI-appeasing tweak --- .../c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py index ea247ee2fa21..8fe263148256 100644 --- a/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py +++ b/superset/migrations/versions/c2acd2cf3df2_alter_type_of_dbs_encrypted_extra.py @@ -23,10 +23,8 @@ """ -from alembic import op - import sqlalchemy as sa - +from alembic import op from sqlalchemy_utils import EncryptedType # revision identifiers, used by Alembic.