From 4aeed9729bd4ea3592ded457b8e39dfb31eec93f Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 18 Apr 2022 11:34:46 -0700 Subject: [PATCH 1/5] fix migration --- ...dbaba_rm_time_range_endpoints_from_qc_3.py | 77 ++++++++++ ...rm_time_range_endpoints_from_qc_3__test.py | 134 ++++++++++++++++++ 2 files changed, 211 insertions(+) create mode 100644 superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py create mode 100644 tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py diff --git a/superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py b/superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py new file mode 100644 index 0000000000000..9be4274a3aef8 --- /dev/null +++ b/superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py @@ -0,0 +1,77 @@ +# 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. +"""rm_time_range_endpoints_from_qc_3 + +Revision ID: 0d07e4fdbaba +Revises: cecc6bf46990 +Create Date: 2022-04-18 11:20:47.390901 + +""" + +# revision identifiers, used by Alembic. +revision = "0d07e4fdbaba" +down_revision = "cecc6bf46990" + +import json + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.ext.declarative import declarative_base + +from superset import db + +Base = declarative_base() + + +class Slice(Base): + __tablename__ = "slices" + id = sa.Column(sa.Integer, primary_key=True) + query_context = sa.Column(sa.Text) + slice_name = sa.Column(sa.String(250)) + + +def upgrade_slice(slc: Slice): + try: + query_context = json.loads(slc.query_context) + except json.decoder.JSONDecodeError: + return + + queries = query_context.get("queries") + + for query in queries: + query.get("extras", {}).pop("time_range_endpoints", None) + query.get("form_data", {}).pop("time_range_endpoints", None) + + slc.query_context = json.dumps(query_context) + + +def upgrade(): + bind = op.get_bind() + session = db.Session(bind=bind) + for slc in ( + session.query(Slice) + .filter(Slice.query_context.like("%time_range_endpoints%")) + .all() + ): + upgrade_slice(slc) + + session.commit() + session.close() + + +def downgrade(): + pass diff --git a/tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py b/tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py new file mode 100644 index 0000000000000..77d228a12b225 --- /dev/null +++ b/tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py @@ -0,0 +1,134 @@ +# 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. +import json + +from superset.migrations.versions.0d07e4fdbaba_rm_time_range_endpoints_from_qc_3 import ( + Slice, + upgrade_slice, +) + +sample_query_context = { + "datasource": {"id": 27, "type": "table"}, + "force": False, + "queries": [ + { + "time_range": "No filter", + "filters": [], + "extras": { + "time_grain_sqla": "P1D", + "time_range_endpoints": ["inclusive", "exclusive"], + "having": "", + "having_druid": [], + "where": "", + }, + "applied_time_extras": {}, + "columns": ["a", "b"], + "orderby": [], + "annotation_layers": [], + "row_limit": 1000, + "timeseries_limit": 0, + "order_desc": True, + "url_params": {}, + "custom_params": {}, + "custom_form_data": {}, + "post_processing": [], + } + ], + "form_data": { + "viz_type": "table", + "datasource": "27__table", + "slice_id": 545, + "url_params": {}, + "time_grain_sqla": "P1D", + "time_range": "No filter", + "query_mode": "raw", + "groupby": [], + "metrics": [], + "all_columns": ["a", "b"], + "percent_metrics": [], + "adhoc_filters": [], + "order_by_cols": [], + "row_limit": 1000, + "server_page_length": 10, + "include_time": False, + "order_desc": True, + "table_timestamp_format": "smart_date", + "show_cell_bars": True, + "color_pn": True, + "extra_form_data": {}, + "force": False, + "result_format": "json", + "result_type": "full", + }, + "result_format": "json", + "result_type": "full", +} + + +sample_query_context = { + "datasource": {"id": 27, "type": "table"}, + "force": False, + "queries": [ + { + "time_range": "No filter", + "filters": [], + "extras": { + "time_grain_sqla": "P1D", + "time_range_endpoints": ["inclusive", "exclusive"], + "having": "", + "having_druid": [], + "where": "", + }, + "applied_time_extras": {}, + "columns": ["a", "b"], + "orderby": [], + "annotation_layers": [], + "row_limit": 1000, + "timeseries_limit": 0, + "order_desc": True, + "url_params": {}, + "custom_params": {}, + "custom_form_data": {}, + "post_processing": [], + } + ], + "form_data": { + "time_range_endpoints": ["inclusive", "exclusive"], + }, + "result_format": "json", + "result_type": "full", +} + + +def test_upgrade(): + slc = Slice(slice_name="FOO", query_context=json.dumps(sample_query_context)) + + upgrade_slice(slc) + + query_context = json.loads(slc.query_context) + queries = query_context.get("queries") + for q in queries: + extras = q.get("extras", {}) + form_data = q.get("form_data", {}) + assert "time_range_endpoints" not in extras + assert "time_range_endpoints" not in form_data + + +def test_upgrade_bad_json(): + slc = Slice(slice_name="FOO", query_context="abc") + + assert None == upgrade_slice(slc) From 32844f7d40ff0a73a1696d071e8fec3a59263610 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 18 Apr 2022 11:57:26 -0700 Subject: [PATCH 2/5] so dumb --- ...baba_rm_time_range_endpoints_from_qc_3.py} | 4 +- .../cecc6bf46990_rm_time_range_endpoints_2.py | 41 +----- ...m_time_range_endpoints_from_qc_3__test.py} | 2 +- ...f46990_rm_time_range_endpoints_2__tests.py | 130 ------------------ 4 files changed, 4 insertions(+), 173 deletions(-) rename superset/migrations/versions/{0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py => ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py} (97%) rename tests/integration_tests/migrations/{0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py => ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py} (98%) delete mode 100644 tests/integration_tests/migrations/cecc6bf46990_rm_time_range_endpoints_2__tests.py diff --git a/superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py similarity index 97% rename from superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py rename to superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py index 9be4274a3aef8..15cc0a3a5641d 100644 --- a/superset/migrations/versions/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3.py +++ b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py @@ -16,14 +16,14 @@ # under the License. """rm_time_range_endpoints_from_qc_3 -Revision ID: 0d07e4fdbaba +Revision ID: ad07e4fdbaba Revises: cecc6bf46990 Create Date: 2022-04-18 11:20:47.390901 """ # revision identifiers, used by Alembic. -revision = "0d07e4fdbaba" +revision = "ad07e4fdbaba" down_revision = "cecc6bf46990" import json diff --git a/superset/migrations/versions/cecc6bf46990_rm_time_range_endpoints_2.py b/superset/migrations/versions/cecc6bf46990_rm_time_range_endpoints_2.py index 20d797ddbaf20..bd2532e88a1c2 100644 --- a/superset/migrations/versions/cecc6bf46990_rm_time_range_endpoints_2.py +++ b/superset/migrations/versions/cecc6bf46990_rm_time_range_endpoints_2.py @@ -26,48 +26,9 @@ revision = "cecc6bf46990" down_revision = "9d8a8d575284" -import json - -import sqlalchemy as sa -from alembic import op -from sqlalchemy.ext.declarative import declarative_base - -from superset import db - -Base = declarative_base() - - -class Slice(Base): - __tablename__ = "slices" - id = sa.Column(sa.Integer, primary_key=True) - query_context = sa.Column(sa.Text) - slice_name = sa.Column(sa.String(250)) - - -def upgrade_slice(slc: Slice): - try: - query_context = json.loads(slc.query_context) - except json.decoder.JSONDecodeError: - return - - queries = query_context.get("queries") - - for query in queries: - query.get("extras", {}).pop("time_range_endpoints", None) - - slc.query_context = json.dumps(query_context) - def upgrade(): - bind = op.get_bind() - session = db.Session(bind=bind) - for slc in session.query(Slice).filter( - Slice.query_context.like("%time_range_endpoints%") - ): - upgrade_slice(slc) - - session.commit() - session.close() + pass def downgrade(): diff --git a/tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py b/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py similarity index 98% rename from tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py rename to tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py index 77d228a12b225..d6efef5891530 100644 --- a/tests/integration_tests/migrations/0d07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py +++ b/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py @@ -16,7 +16,7 @@ # under the License. import json -from superset.migrations.versions.0d07e4fdbaba_rm_time_range_endpoints_from_qc_3 import ( +from superset.migrations.versions.ad07e4fdbaba_rm_time_range_endpoints_from_qc_3 import ( Slice, upgrade_slice, ) diff --git a/tests/integration_tests/migrations/cecc6bf46990_rm_time_range_endpoints_2__tests.py b/tests/integration_tests/migrations/cecc6bf46990_rm_time_range_endpoints_2__tests.py deleted file mode 100644 index 26d9eec0a5e75..0000000000000 --- a/tests/integration_tests/migrations/cecc6bf46990_rm_time_range_endpoints_2__tests.py +++ /dev/null @@ -1,130 +0,0 @@ -# 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. -import json - -from superset.migrations.versions.cecc6bf46990_rm_time_range_endpoints_2 import ( - Slice, - upgrade_slice, -) - -sample_query_context = { - "datasource": {"id": 27, "type": "table"}, - "force": False, - "queries": [ - { - "time_range": "No filter", - "filters": [], - "extras": { - "time_grain_sqla": "P1D", - "time_range_endpoints": ["inclusive", "exclusive"], - "having": "", - "having_druid": [], - "where": "", - }, - "applied_time_extras": {}, - "columns": ["a", "b"], - "orderby": [], - "annotation_layers": [], - "row_limit": 1000, - "timeseries_limit": 0, - "order_desc": True, - "url_params": {}, - "custom_params": {}, - "custom_form_data": {}, - "post_processing": [], - } - ], - "form_data": { - "viz_type": "table", - "datasource": "27__table", - "slice_id": 545, - "url_params": {}, - "time_grain_sqla": "P1D", - "time_range": "No filter", - "query_mode": "raw", - "groupby": [], - "metrics": [], - "all_columns": ["a", "b"], - "percent_metrics": [], - "adhoc_filters": [], - "order_by_cols": [], - "row_limit": 1000, - "server_page_length": 10, - "include_time": False, - "order_desc": True, - "table_timestamp_format": "smart_date", - "show_cell_bars": True, - "color_pn": True, - "extra_form_data": {}, - "force": False, - "result_format": "json", - "result_type": "full", - }, - "result_format": "json", - "result_type": "full", -} - - -sample_query_context = { - "datasource": {"id": 27, "type": "table"}, - "force": False, - "queries": [ - { - "time_range": "No filter", - "filters": [], - "extras": { - "time_grain_sqla": "P1D", - "time_range_endpoints": ["inclusive", "exclusive"], - "having": "", - "having_druid": [], - "where": "", - }, - "applied_time_extras": {}, - "columns": ["a", "b"], - "orderby": [], - "annotation_layers": [], - "row_limit": 1000, - "timeseries_limit": 0, - "order_desc": True, - "url_params": {}, - "custom_params": {}, - "custom_form_data": {}, - "post_processing": [], - } - ], - "form_data": {}, - "result_format": "json", - "result_type": "full", -} - - -def test_upgrade(): - slc = Slice(slice_name="FOO", query_context=json.dumps(sample_query_context)) - - upgrade_slice(slc) - - query_context = json.loads(slc.query_context) - queries = query_context.get("queries") - for q in queries: - extras = q.get("extras", {}) - assert "time_range_endpoints" not in extras - - -def test_upgrade_bad_json(): - slc = Slice(slice_name="FOO", query_context="abc") - - assert None == upgrade_slice(slc) From ce63dfe9a1688fbadd4b41c838c8def9039be0e1 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Mon, 18 Apr 2022 18:42:10 -0700 Subject: [PATCH 3/5] update test --- ...ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py | 12 +++++++++--- ...fdbaba_rm_time_range_endpoints_from_qc_3__test.py | 5 +++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py index 15cc0a3a5641d..e4f3dc7d6a064 100644 --- a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py +++ b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py @@ -51,24 +51,30 @@ def upgrade_slice(slc: Slice): return queries = query_context.get("queries") - + query_context.get("form_data", {}).pop("time_range_endpoints", None) for query in queries: query.get("extras", {}).pop("time_range_endpoints", None) - query.get("form_data", {}).pop("time_range_endpoints", None) slc.query_context = json.dumps(query_context) + return slc + def upgrade(): bind = op.get_bind() session = db.Session(bind=bind) + slices_updated = 0 for slc in ( session.query(Slice) .filter(Slice.query_context.like("%time_range_endpoints%")) .all() ): - upgrade_slice(slc) + updated_slice = upgrade_slice(slc) + if updated_slice: + session.merge(slc) + slices_updated += 1 + print(f"slices updated with no time_range_endpoints: {slices_updated}") session.commit() session.close() diff --git a/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py b/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py index d6efef5891530..f2abfa9766196 100644 --- a/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py +++ b/tests/integration_tests/migrations/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3__test.py @@ -123,9 +123,10 @@ def test_upgrade(): queries = query_context.get("queries") for q in queries: extras = q.get("extras", {}) - form_data = q.get("form_data", {}) assert "time_range_endpoints" not in extras - assert "time_range_endpoints" not in form_data + + form_data = query_context.get("form_data", {}) + assert "time_range_endpoints" not in form_data def test_upgrade_bad_json(): From 5f43c5b3f6d22ab78bc5de5133760bb020438168 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Tue, 19 Apr 2022 08:18:59 -0700 Subject: [PATCH 4/5] add code change --- .../ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py index e4f3dc7d6a064..86eba12744360 100644 --- a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py +++ b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py @@ -50,10 +50,13 @@ def upgrade_slice(slc: Slice): except json.decoder.JSONDecodeError: return - queries = query_context.get("queries") query_context.get("form_data", {}).pop("time_range_endpoints", None) - for query in queries: - query.get("extras", {}).pop("time_range_endpoints", None) + + if query_context.get("queries"): + queries = query_context["queries"] + for query in queries: + if query.get("extras", None).get("time_range_endpoints"): + del query["extras"]["time_range_endpoints"] slc.query_context = json.dumps(query_context) From eb324a0f229db8117f2d104d96f277b0f85068f5 Mon Sep 17 00:00:00 2001 From: hughhhh Date: Tue, 19 Apr 2022 08:32:58 -0700 Subject: [PATCH 5/5] retest --- .../ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py index 86eba12744360..30efb1a083fc2 100644 --- a/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py +++ b/superset/migrations/versions/ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py @@ -55,8 +55,7 @@ def upgrade_slice(slc: Slice): if query_context.get("queries"): queries = query_context["queries"] for query in queries: - if query.get("extras", None).get("time_range_endpoints"): - del query["extras"]["time_range_endpoints"] + query.get("extras", {}).pop("time_range_endpoints", None) slc.query_context = json.dumps(query_context) @@ -74,7 +73,6 @@ def upgrade(): ): updated_slice = upgrade_slice(slc) if updated_slice: - session.merge(slc) slices_updated += 1 print(f"slices updated with no time_range_endpoints: {slices_updated}")