From 46c681f255f8d243cbc49e5b50362e48c7818f7a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 2 Jun 2020 12:32:14 +0100 Subject: [PATCH 01/17] feat(api): bump marshmallow and FAB to version 3 --- requirements.txt | 6 +-- setup.py | 2 +- superset/charts/api.py | 35 +++++++------ superset/charts/schemas.py | 15 +++--- superset/commands/exceptions.py | 4 +- superset/dashboards/api.py | 28 ++++++----- superset/dashboards/commands/exceptions.py | 2 +- superset/dashboards/schemas.py | 14 ++++-- superset/datasets/api.py | 23 +++++---- superset/datasets/commands/exceptions.py | 42 ++++++++-------- tests/base_api_tests.py | 58 ++++++++++++++++------ tests/charts/api_tests.py | 6 ++- tests/dashboards/api_tests.py | 4 +- tests/datasets/api_tests.py | 13 +++++ tests/query_context_tests.py | 13 +++-- 15 files changed, 160 insertions(+), 105 deletions(-) diff --git a/requirements.txt b/requirements.txt index b79d2a7c1c00..db52674c63cd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ # alembic==1.4.2 # via flask-migrate amqp==2.5.2 # via kombu -apispec[yaml]==1.3.3 # via flask-appbuilder +apispec[yaml]==3.3.0 # via flask-appbuilder attrs==19.3.0 # via jsonschema babel==2.8.0 # via flask-babel backoff==1.10.0 # via apache-superset (setup.py) @@ -26,7 +26,7 @@ decorator==4.4.2 # via retry defusedxml==0.6.0 # via python3-openid dnspython==1.16.0 # via email-validator email-validator==1.1.0 # via flask-appbuilder -flask-appbuilder==2.3.4 # via apache-superset (setup.py) +flask-appbuilder==3.0.0rc1 # via apache-superset (setup.py) flask-babel==1.0.0 # via flask-appbuilder flask-caching==1.8.0 # via apache-superset (setup.py) flask-compress==1.5.0 # via apache-superset (setup.py) @@ -54,7 +54,7 @@ markdown==3.2.2 # via apache-superset (setup.py) markupsafe==1.1.1 # via jinja2, mako, wtforms marshmallow-enum==1.5.1 # via flask-appbuilder marshmallow-sqlalchemy==0.23.0 # via flask-appbuilder -marshmallow==2.21.0 # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy +marshmallow==3.6.0 # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy msgpack==1.0.0 # via apache-superset (setup.py) numpy==1.18.4 # via pandas, pyarrow packaging==20.3 # via bleach diff --git a/setup.py b/setup.py index ed8176cd00a8..cc26c57d47da 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,7 @@ def get_git_sha(): "cryptography>=2.4.2", "dataclasses<0.7", "flask>=1.1.0, <2.0.0", - "flask-appbuilder>=2.3.4, <2.4.0", + "flask-appbuilder==3.0.0rc1", "flask-caching", "flask-compress", "flask-talisman", diff --git a/superset/charts/api.py b/superset/charts/api.py index 19de171ce841..4e4980753629 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -22,6 +22,7 @@ from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import gettext as _, ngettext +from marshmallow import ValidationError from werkzeug.wrappers import Response as WerkzeugResponse from werkzeug.wsgi import FileWrapper @@ -207,13 +208,14 @@ def post(self) -> Response: """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.add_model_schema.load(request.json) + try: + item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - new_model = CreateChartCommand(g.user, item.data).run() - return self.response(201, id=new_model.id, result=item.data) + new_model = CreateChartCommand(g.user, item).run() + return self.response(201, id=new_model.id, result=item) except ChartInvalidError as ex: return self.response_422(message=ex.normalized_messages()) except ChartCreateFailedError as ex: @@ -271,13 +273,14 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.edit_model_schema.load(request.json) + try: + item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - changed_model = UpdateChartCommand(g.user, pk, item.data).run() - return self.response(200, id=changed_model.id, result=item.data) + changed_model = UpdateChartCommand(g.user, pk, item).run() + return self.response(200, id=changed_model.id, result=item) except ChartNotFoundError: return self.response_404() except ChartForbiddenError: @@ -430,17 +433,17 @@ def data(self) -> Response: $ref: '#/components/responses/400' 500: $ref: '#/components/responses/500' - """ + """ if not request.is_json: return self.response_400(message="Request is not JSON") try: - query_context, errors = ChartDataQueryContextSchema().load(request.json) - if errors: - return self.response_400( - message=_("Request is incorrect: %(error)s", error=errors) - ) + query_context = ChartDataQueryContextSchema().load(request.json) except KeyError: return self.response_400(message="Request is incorrect") + except ValidationError as err: + return self.response_400( + _("Request is incorrect: %(error)s", error=err.messages) + ) try: security_manager.assert_query_context_permission(query_context) except SupersetSecurityException: diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 609a868fea18..d25bf49fb995 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -233,8 +233,7 @@ def __init__(self) -> None: class ChartDataPostProcessingOperationOptionsSchema(Schema): - def __init__(self, *args: Any, **kwargs: Any) -> None: - super().__init__(*args, **kwargs) + pass class ChartDataAggregateOptionsSchema(ChartDataPostProcessingOperationOptionsSchema): @@ -661,6 +660,10 @@ class ChartDataQueryObjectSchema(Schema): timeseries_limit = fields.Integer( description="Maximum row count for timeseries queries. Default: `0`", ) + timeseries_limit_metric = fields.Integer( + description="Maximum row count for timeseries queries. Default: `0`", + allow_none=True, + ) row_limit = fields.Integer( description='Maximum row count. Default: `config["ROW_LIMIT"]`', ) @@ -713,20 +716,20 @@ class ChartDataQueryContextSchema(Schema): ) result_type = fields.String( description="Type of results to return", - validate=validate.OneOf(choices=("full", "query", "results", "samples")), + validate=validate.OneOf(choices=("query", "results", "samples")), ) result_format = fields.String( description="Format of result payload", validate=validate.OneOf(choices=("json", "csv")), ) - # pylint: disable=no-self-use + # pylint: disable=no-self-use,unused-argument @post_load - def make_query_context(self, data: Dict[str, Any]) -> QueryContext: + def make_query_context(self, data: Dict[str, Any], **kwargs) -> QueryContext: query_context = QueryContext(**data) return query_context - # pylint: enable=no-self-use + # pylint: enable=no-self-use,unused-argument class ChartDataResponseResult(Schema): diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py index cf67ea922715..f5086725fb6c 100644 --- a/superset/commands/exceptions.py +++ b/superset/commands/exceptions.py @@ -77,11 +77,11 @@ class OwnersNotFoundValidationError(ValidationError): status = 422 def __init__(self) -> None: - super().__init__(_("Owners are invalid"), field_names=["owners"]) + super().__init__([_("Owners are invalid")], field_name="owners") class DatasourceNotFoundValidationError(ValidationError): status = 404 def __init__(self) -> None: - super().__init__(_("Datasource does not exist"), field_names=["datasource_id"]) + super().__init__([_("Datasource does not exist")], field_name="datasource_id") diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 21d4e7959fe4..e52bd68caaf9 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -21,6 +21,7 @@ from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext +from marshmallow import ValidationError from werkzeug.wrappers import Response as WerkzeugResponse from werkzeug.wsgi import FileWrapper @@ -117,7 +118,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): "owners.first_name", "owners.last_name", ] - edit_columns = [ + add_columns = [ "dashboard_title", "slug", "owners", @@ -126,9 +127,10 @@ class DashboardRestApi(BaseSupersetModelRestApi): "json_metadata", "published", ] + edit_columns = add_columns + search_columns = ("dashboard_title", "slug", "owners", "published") search_filters = {"dashboard_title": [DashboardTitleOrSlugFilter]} - add_columns = edit_columns base_order = ("changed_on", "desc") add_model_schema = DashboardPostSchema() @@ -196,13 +198,14 @@ def post(self) -> Response: """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.add_model_schema.load(request.json) + try: + item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - new_model = CreateDashboardCommand(g.user, item.data).run() - return self.response(201, id=new_model.id, result=item.data) + new_model = CreateDashboardCommand(g.user, item).run() + return self.response(201, id=new_model.id, result=item) except DashboardInvalidError as ex: return self.response_422(message=ex.normalized_messages()) except DashboardCreateFailedError as ex: @@ -260,13 +263,14 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.edit_model_schema.load(request.json) + try: + item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - changed_model = UpdateDashboardCommand(g.user, pk, item.data).run() - return self.response(200, id=changed_model.id, result=item.data) + changed_model = UpdateDashboardCommand(g.user, pk, item).run() + return self.response(200, id=changed_model.id, result=item) except DashboardNotFoundError: return self.response_404() except DashboardForbiddenError: diff --git a/superset/dashboards/commands/exceptions.py b/superset/dashboards/commands/exceptions.py index 13b0d5b74bc9..c6f78ee8b79b 100644 --- a/superset/dashboards/commands/exceptions.py +++ b/superset/dashboards/commands/exceptions.py @@ -33,7 +33,7 @@ class DashboardSlugExistsValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__(_("Must be unique"), field_names=["slug"]) + super().__init__([_("Must be unique")], field_name="slug") class DashboardInvalidError(CommandInvalidError): diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 0d8c60ad170b..8ae6bf1ea62b 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -18,7 +18,7 @@ import re from typing import Any, Dict, Union -from marshmallow import fields, pre_load, Schema +from marshmallow import fields, post_load, Schema from marshmallow.validate import Length, ValidationError from superset.exceptions import SupersetException @@ -92,7 +92,7 @@ def validate_json_metadata(value: Union[bytes, bytearray, str]) -> None: value_obj = json.loads(value) except json.decoder.JSONDecodeError: raise ValidationError("JSON not valid") - errors = DashboardJSONMetadataSchema(strict=True).validate(value_obj, partial=False) + errors = DashboardJSONMetadataSchema().validate(value_obj, partial=False) if errors: raise ValidationError(errors) @@ -110,12 +110,16 @@ class DashboardJSONMetadataSchema(Schema): class BaseDashboardSchema(Schema): - @pre_load - def pre_load(self, data: Dict[str, Any]) -> None: # pylint: disable=no-self-use + # pylint: disable=no-self-use,unused-argument + @post_load + def pre_load(self, data: Dict[str, Any], **kwargs) -> None: if data.get("slug"): data["slug"] = data["slug"].strip() data["slug"] = data["slug"].replace(" ", "-") data["slug"] = re.sub(r"[^\w\-]+", "", data["slug"]) + return data + + # pylint: disable=no-self-use,unused-argument class DashboardPostSchema(BaseDashboardSchema): @@ -133,7 +137,7 @@ class DashboardPostSchema(BaseDashboardSchema): ) css = fields.String() json_metadata = fields.String( - description=json_metadata_description, validate=validate_json_metadata + description=json_metadata_description, validate=validate_json_metadata, ) published = fields.Boolean(description=published_description) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 7452b662b7db..57026f24be32 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -21,6 +21,7 @@ from flask import g, request, Response from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface +from marshmallow import ValidationError from superset.connectors.sqla.models import SqlaTable from superset.constants import RouteMethod @@ -171,13 +172,14 @@ def post(self) -> Response: """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.add_model_schema.load(request.json) + try: + item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - new_model = CreateDatasetCommand(g.user, item.data).run() - return self.response(201, id=new_model.id, result=item.data) + new_model = CreateDatasetCommand(g.user, item).run() + return self.response(201, id=new_model.id, result=item) except DatasetInvalidError as ex: return self.response_422(message=ex.normalized_messages()) except DatasetCreateFailedError as ex: @@ -235,13 +237,14 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ """ if not request.is_json: return self.response_400(message="Request is not JSON") - item = self.edit_model_schema.load(request.json) + try: + item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - if item.errors: - return self.response_400(message=item.errors) + except ValidationError as err: + return self.response_400(message=err.messages) try: - changed_model = UpdateDatasetCommand(g.user, pk, item.data).run() - return self.response(200, id=changed_model.id, result=item.data) + changed_model = UpdateDatasetCommand(g.user, pk, item).run() + return self.response(200, id=changed_model.id, result=item) except DatasetNotFoundError: return self.response_404() except DatasetForbiddenError: diff --git a/superset/datasets/commands/exceptions.py b/superset/datasets/commands/exceptions.py index 69bd7cd7d3e4..b651b6feb9fb 100644 --- a/superset/datasets/commands/exceptions.py +++ b/superset/datasets/commands/exceptions.py @@ -34,7 +34,7 @@ class DatabaseNotFoundValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__(_("Database does not exist"), field_names=["database"]) + super().__init__([_("Database does not exist")], field_name="database") class DatabaseChangeValidationError(ValidationError): @@ -43,7 +43,7 @@ class DatabaseChangeValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__(_("Database not allowed to change"), field_names=["database"]) + super().__init__([_("Database not allowed to change")], field_name="database") class DatasetExistsValidationError(ValidationError): @@ -53,7 +53,7 @@ class DatasetExistsValidationError(ValidationError): def __init__(self, table_name: str) -> None: super().__init__( - get_datasource_exist_error_msg(table_name), field_names=["table_name"] + get_datasource_exist_error_msg(table_name), field_name="table_name" ) @@ -63,7 +63,7 @@ class DatasetColumnNotFoundValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__(_("One or more columns do not exist"), field_names=["columns"]) + super().__init__([_("One or more columns do not exist")], field_name="columns") class DatasetColumnsDuplicateValidationError(ValidationError): @@ -73,7 +73,7 @@ class DatasetColumnsDuplicateValidationError(ValidationError): def __init__(self) -> None: super().__init__( - _("One or more columns are duplicated"), field_names=["columns"] + [_("One or more columns are duplicated")], field_name="columns" ) @@ -83,9 +83,7 @@ class DatasetColumnsExistsValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__( - _("One or more columns already exist"), field_names=["columns"] - ) + super().__init__([_("One or more columns already exist")], field_name="columns") class DatasetMetricsNotFoundValidationError(ValidationError): @@ -94,7 +92,7 @@ class DatasetMetricsNotFoundValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__(_("One or more metrics do not exist"), field_names=["metrics"]) + super().__init__([_("One or more metrics do not exist")], field_name="metrics") class DatasetMetricsDuplicateValidationError(ValidationError): @@ -104,7 +102,7 @@ class DatasetMetricsDuplicateValidationError(ValidationError): def __init__(self) -> None: super().__init__( - _("One or more metrics are duplicated"), field_names=["metrics"] + [_("One or more metrics are duplicated")], field_name="metrics" ) @@ -114,9 +112,7 @@ class DatasetMetricsExistsValidationError(ValidationError): """ def __init__(self) -> None: - super().__init__( - _("One or more metrics already exist"), field_names=["metrics"] - ) + super().__init__([_("One or more metrics already exist")], field_name="metrics") class TableNotFoundValidationError(ValidationError): @@ -126,20 +122,22 @@ class TableNotFoundValidationError(ValidationError): def __init__(self, table_name: str) -> None: super().__init__( - _( - "Table [%(table_name)s] could not be found, " - "please double check your " - "database connection, schema, and " - "table name", - table_name=table_name, - ), - field_names=["table_name"], + [ + _( + "Table [%(table_name)s] could not be found, " + "please double check your " + "database connection, schema, and " + "table name", + table_name=table_name, + ) + ], + field_name="table_name", ) class OwnersNotFoundValidationError(ValidationError): def __init__(self) -> None: - super().__init__(_("Owners are invalid"), field_names=["owners"]) + super().__init__([_("Owners are invalid")], field_name="owners") class DatasetNotFoundError(CommandException): diff --git a/tests/base_api_tests.py b/tests/base_api_tests.py index 112211abb50e..2d2e99a191f6 100644 --- a/tests/base_api_tests.py +++ b/tests/base_api_tests.py @@ -51,7 +51,10 @@ class Model1Api(BaseSupersetModelRestApi): class BaseModelRestApiTests(SupersetTestCase): def test_default_missing_declaration_get(self): """ - API: Test default missing declaration on get + API: Test default missing declaration on get + + We want to make sure that not declared list_columns will + not render all columns by default but just the model's pk """ # Check get list response self.login(username="admin") @@ -73,6 +76,12 @@ def test_default_missing_declaration_get(self): self.assertEqual(list(response["result"].keys()), ["id"]) def test_default_missing_declaration_put_spec(self): + """ + API: Test default missing declaration on put openapi spec + + We want to make sure that not declared edit_columns will + not render all columns by default but just the model's pk + """ self.login(username="admin") uri = "api/v1/_openapi" rv = self.client.get(uri) @@ -91,6 +100,12 @@ def test_default_missing_declaration_put_spec(self): ) def test_default_missing_declaration_post(self): + """ + API: Test default missing declaration on post + + We want to make sure that not declared add_columns will + not accept all columns by default + """ dashboard_data = { "dashboard_title": "title1", "slug": "slug1", @@ -102,30 +117,41 @@ def test_default_missing_declaration_post(self): self.login(username="admin") uri = "api/v1/model1api/" rv = self.client.post(uri, json=dashboard_data) - # dashboard model accepts all fields are null - self.assertEqual(rv.status_code, 201) response = json.loads(rv.data.decode("utf-8")) - self.assertEqual(list(response["result"].keys()), ["id"]) - model = db.session.query(Dashboard).get(response["id"]) - self.assertEqual(model.dashboard_title, None) - self.assertEqual(model.slug, None) - self.assertEqual(model.position_json, None) - self.assertEqual(model.json_metadata, None) - db.session.delete(model) - db.session.commit() + self.assertEqual(rv.status_code, 422) + expected_response = { + "message": { + "css": ["Unknown field."], + "dashboard_title": ["Unknown field."], + "json_metadata": ["Unknown field."], + "position_json": ["Unknown field."], + "published": ["Unknown field."], + "slug": ["Unknown field."], + } + } + self.assertEqual(response, expected_response) def test_default_missing_declaration_put(self): + """ + API: Test default missing declaration on put + + We want to make sure that not declared edit_columns will + not accept all columns by default + """ dashboard = db.session.query(Dashboard).first() dashboard_data = {"dashboard_title": "CHANGED", "slug": "CHANGED"} self.login(username="admin") uri = f"api/v1/model1api/{dashboard.id}" rv = self.client.put(uri, json=dashboard_data) - # dashboard model accepts all fields are null - self.assertEqual(rv.status_code, 200) response = json.loads(rv.data.decode("utf-8")) - changed_dashboard = db.session.query(Dashboard).get(dashboard.id) - self.assertNotEqual(changed_dashboard.dashboard_title, "CHANGED") - self.assertNotEqual(changed_dashboard.slug, "CHANGED") + self.assertEqual(rv.status_code, 422) + expected_response = { + "message": { + "dashboard_title": ["Unknown field."], + "slug": ["Unknown field."], + } + } + self.assertEqual(response, expected_response) class ApiOwnersTestCaseMixin: diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index 3e9769c63558..ec109cb6c397 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -336,7 +336,8 @@ def test_create_chart_validate_datasource(self): self.assertEqual(rv.status_code, 400) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( - response, {"message": {"datasource_type": ["Not a valid choice."]}} + response, + {"message": {"datasource_type": ["Must be one of: druid, table, view."]}}, ) chart_data = { "slice_name": "title1", @@ -442,7 +443,8 @@ def test_update_chart_validate_datasource(self): self.assertEqual(rv.status_code, 400) response = json.loads(rv.data.decode("utf-8")) self.assertEqual( - response, {"message": {"datasource_type": ["Not a valid choice."]}} + response, + {"message": {"datasource_type": ["Must be one of: druid, table, view."]}}, ) chart_data = {"datasource_id": 0, "datasource_type": "table"} uri = f"api/v1/chart/{chart.id}" diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index 23cdd882faf9..2d5a838c0c83 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -40,7 +40,7 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): "slug": "slug1_changed", "position_json": '{"b": "B"}', "css": "css_changed", - "json_metadata": '{"a": "A"}', + "json_metadata": '{"refresh_frequency": 30}', "published": False, } @@ -476,7 +476,7 @@ def test_create_dashboard(self): "owners": [admin_id], "position_json": '{"a": "A"}', "css": "css", - "json_metadata": '{"b": "B"}', + "json_metadata": '{"refresh_frequency": 30}', "published": True, } self.login(username="admin") diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py index 47d98e6490d1..edfe792267f5 100644 --- a/tests/datasets/api_tests.py +++ b/tests/datasets/api_tests.py @@ -367,6 +367,13 @@ def test_update_dataset_create_column(self): self.login(username="admin") rv = self.get_assert_metric(uri, "get") data = json.loads(rv.data.decode("utf-8")) + + for column in data["result"]["columns"]: + column.pop("changed_on", None) + column.pop("created_on", None) + column.pop("changed_on", None) + column.pop("changed_on", None) + data["result"]["columns"].append(new_column_data) rv = self.client.put(uri, json={"columns": data["result"]["columns"]}) @@ -400,6 +407,12 @@ def test_update_dataset_update_column(self): # Get current cols and alter one rv = self.get_assert_metric(uri, "get") resp_columns = json.loads(rv.data.decode("utf-8"))["result"]["columns"] + for column in resp_columns: + column.pop("changed_on", None) + column.pop("created_on", None) + column.pop("changed_on", None) + column.pop("changed_on", None) + resp_columns[0]["groupby"] = False resp_columns[0]["filterable"] = False v = self.client.put(uri, json={"columns": resp_columns}) diff --git a/tests/query_context_tests.py b/tests/query_context_tests.py index 310df012dca0..19dd0ef424cd 100644 --- a/tests/query_context_tests.py +++ b/tests/query_context_tests.py @@ -20,8 +20,8 @@ from superset.common.query_context import QueryContext from superset.connectors.connector_registry import ConnectorRegistry from superset.utils.core import ( - ChartDataResultFormat, - ChartDataResultType, + ChartDataResponseFormat, + ChartDataResponseType, TimeRangeEndpoint, ) from tests.base_tests import SupersetTestCase @@ -39,8 +39,7 @@ def test_schema_deserialization(self): payload = get_query_context( table.name, table.id, table.type, add_postprocessing_operations=True ) - query_context, errors = ChartDataQueryContextSchema().load(payload) - self.assertDictEqual(errors, {}) + query_context = ChartDataQueryContextSchema().load(payload) self.assertEqual(len(query_context.queries), len(payload["queries"])) for query_idx, query in enumerate(query_context.queries): payload_query = payload["queries"][query_idx] @@ -144,7 +143,7 @@ def test_csv_response_format(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["result_format"] = ChartDataResultFormat.CSV.value + payload["response_format"] = ChartDataResponseFormat.CSV.value payload["queries"][0]["row_limit"] = 10 query_context = QueryContext(**payload) responses = query_context.get_payload() @@ -161,7 +160,7 @@ def test_samples_response_type(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["result_type"] = ChartDataResultType.SAMPLES.value + payload["response_type"] = ChartDataResponseType.SAMPLES.value payload["queries"][0]["row_limit"] = 5 query_context = QueryContext(**payload) responses = query_context.get_payload() @@ -179,7 +178,7 @@ def test_query_response_type(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["result_type"] = ChartDataResultType.QUERY.value + payload["response_type"] = ChartDataResponseType.QUERY.value query_context = QueryContext(**payload) responses = query_context.get_payload() self.assertEqual(len(responses), 1) From 377cd26c3f3571f0b609883cfc46ef10a1135c2a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 2 Jun 2020 13:06:36 +0100 Subject: [PATCH 02/17] revert query context tests changes --- tests/query_context_tests.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/query_context_tests.py b/tests/query_context_tests.py index 19dd0ef424cd..e165e4450092 100644 --- a/tests/query_context_tests.py +++ b/tests/query_context_tests.py @@ -20,8 +20,8 @@ from superset.common.query_context import QueryContext from superset.connectors.connector_registry import ConnectorRegistry from superset.utils.core import ( - ChartDataResponseFormat, - ChartDataResponseType, + ChartDataResultFormat, + ChartDataResultType, TimeRangeEndpoint, ) from tests.base_tests import SupersetTestCase @@ -143,7 +143,7 @@ def test_csv_response_format(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["response_format"] = ChartDataResponseFormat.CSV.value + payload["result_format"] = ChartDataResultFormat.CSV.value payload["queries"][0]["row_limit"] = 10 query_context = QueryContext(**payload) responses = query_context.get_payload() @@ -160,7 +160,7 @@ def test_samples_response_type(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["response_type"] = ChartDataResponseType.SAMPLES.value + payload["result_type"] = ChartDataResultType.SAMPLES.value payload["queries"][0]["row_limit"] = 5 query_context = QueryContext(**payload) responses = query_context.get_payload() @@ -178,7 +178,7 @@ def test_query_response_type(self): table_name = "birth_names" table = self.get_table_by_name(table_name) payload = get_query_context(table.name, table.id, table.type) - payload["response_type"] = ChartDataResponseType.QUERY.value + payload["result_type"] = ChartDataResultType.QUERY.value query_context = QueryContext(**payload) responses = query_context.get_payload() self.assertEqual(len(responses), 1) From ce7cab525c70f1d400e08244e0a41ab6e679c194 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 2 Jun 2020 13:40:23 +0100 Subject: [PATCH 03/17] obey mypy --- superset/charts/commands/update.py | 2 +- superset/charts/schemas.py | 2 +- superset/dashboards/commands/create.py | 2 +- superset/dashboards/schemas.py | 2 +- superset/datasets/commands/create.py | 2 +- superset/datasets/commands/update.py | 2 +- superset/views/base_api.py | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/superset/charts/commands/update.py b/superset/charts/commands/update.py index 21c236ca9621..72df6459df30 100644 --- a/superset/charts/commands/update.py +++ b/superset/charts/commands/update.py @@ -58,7 +58,7 @@ def run(self) -> Model: return chart def validate(self) -> None: - exceptions = list() + exceptions: List[ValidationError] = list() dashboard_ids = self._properties.get("dashboards", []) owner_ids: Optional[List[int]] = self._properties.get("owners") diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index d25bf49fb995..414881ca0dcb 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -725,7 +725,7 @@ class ChartDataQueryContextSchema(Schema): # pylint: disable=no-self-use,unused-argument @post_load - def make_query_context(self, data: Dict[str, Any], **kwargs) -> QueryContext: + def make_query_context(self, data: Dict[str, Any], **kwargs: Any) -> QueryContext: query_context = QueryContext(**data) return query_context diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py index 0aa1241fdad2..ee32563e7c09 100644 --- a/superset/dashboards/commands/create.py +++ b/superset/dashboards/commands/create.py @@ -50,7 +50,7 @@ def run(self) -> Model: return dashboard def validate(self) -> None: - exceptions = list() + exceptions: List[ValidationError] = list() owner_ids: Optional[List[int]] = self._properties.get("owners") slug: str = self._properties.get("slug", "") diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 8ae6bf1ea62b..9ce98f277c19 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -112,7 +112,7 @@ class DashboardJSONMetadataSchema(Schema): class BaseDashboardSchema(Schema): # pylint: disable=no-self-use,unused-argument @post_load - def pre_load(self, data: Dict[str, Any], **kwargs) -> None: + def pre_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: if data.get("slug"): data["slug"] = data["slug"].strip() data["slug"] = data["slug"].replace(" ", "-") diff --git a/superset/datasets/commands/create.py b/superset/datasets/commands/create.py index 3114a4f005d7..c7cd03a5764e 100644 --- a/superset/datasets/commands/create.py +++ b/superset/datasets/commands/create.py @@ -67,7 +67,7 @@ def run(self) -> Model: return dataset def validate(self) -> None: - exceptions = list() + exceptions: List[ValidationError] = list() database_id = self._properties["database"] table_name = self._properties["table_name"] schema = self._properties.get("schema", "") diff --git a/superset/datasets/commands/update.py b/superset/datasets/commands/update.py index c7f70dd16cc6..a26a9623e6a1 100644 --- a/superset/datasets/commands/update.py +++ b/superset/datasets/commands/update.py @@ -66,7 +66,7 @@ def run(self) -> Model: raise DatasetUpdateFailedError() def validate(self) -> None: - exceptions = list() + exceptions: List[ValidationError] = list() owner_ids: Optional[List[int]] = self._properties.get("owners") # Validate/populate model exists self._model = DatasetDAO.find_by_id(self._model_id) diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 5675506dc55f..27fbbbcc3bc3 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -111,7 +111,7 @@ class BaseSupersetModelRestApi(ModelRestApi): """ # pylint: disable=pointless-string-statement allowed_rel_fields: Set[str] = set() - openapi_spec_component_schemas: Tuple[Schema, ...] = tuple() + openapi_spec_component_schemas: Tuple[Type[Schema], ...] = tuple() """ Add extra schemas to the OpenAPI component schemas section """ # pylint: disable=pointless-string-statement @@ -124,7 +124,7 @@ def add_apispec_components(self, api_spec: APISpec) -> None: for schema in self.openapi_spec_component_schemas: api_spec.components.schema( - schema.__name__, schema=schema, + schema.__class__.__name__, schema=schema, ) super().add_apispec_components(api_spec) From 18522ce0025c787c860801f7542f53ac6ba5e17a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 2 Jun 2020 13:49:02 +0100 Subject: [PATCH 04/17] fix tests --- superset/dashboards/schemas.py | 2 +- superset/views/base_api.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 9ce98f277c19..258770586639 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -112,7 +112,7 @@ class DashboardJSONMetadataSchema(Schema): class BaseDashboardSchema(Schema): # pylint: disable=no-self-use,unused-argument @post_load - def pre_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: + def post_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: if data.get("slug"): data["slug"] = data["slug"].strip() data["slug"] = data["slug"].replace(" ", "-") diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 27fbbbcc3bc3..2307f99e0a01 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -124,7 +124,7 @@ def add_apispec_components(self, api_spec: APISpec) -> None: for schema in self.openapi_spec_component_schemas: api_spec.components.schema( - schema.__class__.__name__, schema=schema, + schema.__name__, schema=schema, ) super().add_apispec_components(api_spec) From 7a05ed95ccaada10bcefe37508e46d54c372cb05 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 2 Jun 2020 14:05:03 +0100 Subject: [PATCH 05/17] ignore types that collide with marshmallow --- superset/charts/schemas.py | 2 +- superset/commands/exceptions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 414881ca0dcb..a0320976ac11 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -355,7 +355,7 @@ class ChartDataSelectOptionsSchema(ChartDataPostProcessingOperationOptionsSchema "referenced here.", example=["country", "gender", "age"], ) - exclude = fields.List( + exclude = fields.List( # type: ignore fields.String(), description="Columns to exclude from selection.", example=["my_temp_column"], diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py index f5086725fb6c..d470043bb38b 100644 --- a/superset/commands/exceptions.py +++ b/superset/commands/exceptions.py @@ -49,7 +49,7 @@ def add_list(self, exceptions: List[ValidationError]) -> None: def normalized_messages(self) -> Dict[Any, Any]: errors: Dict[Any, Any] = {} for exception in self._invalid_exceptions: - errors.update(exception.normalized_messages()) + errors.update(exception.normalized_messages()) # type: ignore return errors From 875d8eb11ed1c366ae410c1f071754a91d64a930 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 1 Jul 2020 11:11:44 +0100 Subject: [PATCH 06/17] preparing for RC2 --- superset/charts/api.py | 3 +++ superset/datasets/api.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/superset/charts/api.py b/superset/charts/api.py index 4e4980753629..2be697eb05cf 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -96,6 +96,8 @@ class ChartRestApi(BaseSupersetModelRestApi): "params", "cache_timeout", ] + show_select_columns = show_columns + ["table.id"] + list_columns = [ "id", "slice_name", @@ -118,6 +120,7 @@ class ChartRestApi(BaseSupersetModelRestApi): "params", "cache_timeout", ] + order_columns = [ "slice_name", "viz_type", diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 57026f24be32..aa15e8e78c20 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -61,7 +61,6 @@ class DatasetRestApi(BaseSupersetModelRestApi): resource_name = "dataset" allow_browser_login = True - class_permission_name = "TableModelView" include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | { RouteMethod.EXPORT, From 1e68ad79dec0e7ca30be89443498c93993077922 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 09:56:11 +0100 Subject: [PATCH 07/17] fix tests for marshmallow 3 --- tests/charts/schema_tests.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/charts/schema_tests.py b/tests/charts/schema_tests.py index 4c998fc27abf..f6b4c3430a49 100644 --- a/tests/charts/schema_tests.py +++ b/tests/charts/schema_tests.py @@ -18,6 +18,7 @@ """Unit tests for Superset""" from typing import Any, Dict, Tuple +from marshmallow import ValidationError from tests.test_app import app from superset.charts.schemas import ChartDataQueryContextSchema from superset.common.query_context import QueryContext @@ -48,8 +49,7 @@ def test_query_context_limit_and_offset(self): # Valid limit and offset payload["queries"][0]["row_limit"] = 100 payload["queries"][0]["row_offset"] = 200 - query_context, errors = ChartDataQueryContextSchema().load(payload) - self.assertEqual(errors, {}) + query_context= ChartDataQueryContextSchema().load(payload) query_object = query_context.queries[0] self.assertEqual(query_object.row_limit, 100) self.assertEqual(query_object.row_offset, 200) @@ -57,9 +57,11 @@ def test_query_context_limit_and_offset(self): # too low limit and offset payload["queries"][0]["row_limit"] = 0 payload["queries"][0]["row_offset"] = -1 - query_context, errors = ChartDataQueryContextSchema().load(payload) - self.assertIn("row_limit", errors["queries"][0]) - self.assertIn("row_offset", errors["queries"][0]) + try: + query_context = ChartDataQueryContextSchema().load(payload) + except ValidationError as errors: + self.assertIn("row_limit", errors["queries"][0]) + self.assertIn("row_offset", errors["queries"][0]) def test_query_context_null_timegrain(self): self.login(username="admin") @@ -68,5 +70,4 @@ def test_query_context_null_timegrain(self): payload = get_query_context(table.name, table.id, table.type) payload["queries"][0]["extras"]["time_grain_sqla"] = None - _, errors = ChartDataQueryContextSchema().load(payload) - self.assertEqual(errors, {}) + _ = ChartDataQueryContextSchema().load(payload) From 7e046a8824af7f2b70171d74ede5506eb9d47343 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 10:03:59 +0100 Subject: [PATCH 08/17] typing fixes for marshmallow --- superset/views/base_schemas.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/superset/views/base_schemas.py b/superset/views/base_schemas.py index 87a9190596eb..659ec3fdfcc1 100644 --- a/superset/views/base_schemas.py +++ b/superset/views/base_schemas.py @@ -52,11 +52,15 @@ def load( # pylint: disable=arguments-differ self, data: Union[Mapping[str, Any], Iterable[Mapping[str, Any]]], many: Optional[bool] = None, - partial: Optional[Union[bool, Sequence[str], Set[str]]] = None, + partial: Union[bool, Sequence[str], Set[str], None] = None, instance: Optional[Model] = None, **kwargs: Any, ) -> Any: self.instance = instance + if many is None: + many = False + if partial is None: + partial = False return super().load(data, many=many, partial=partial, **kwargs) @post_load From 7e50e0cf87eb7db728a6abaedfdb32fe9b7e7a3f Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 10:40:35 +0100 Subject: [PATCH 09/17] fix tests and black --- tests/charts/schema_tests.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/charts/schema_tests.py b/tests/charts/schema_tests.py index f6b4c3430a49..eaee3e95c3e3 100644 --- a/tests/charts/schema_tests.py +++ b/tests/charts/schema_tests.py @@ -40,8 +40,7 @@ def test_query_context_limit_and_offset(self): # Use defaults payload["queries"][0].pop("row_limit", None) payload["queries"][0].pop("row_offset", None) - query_context, errors = load_query_context(payload) - self.assertEqual(errors, {}) + query_context = load_query_context(payload) query_object = query_context.queries[0] self.assertEqual(query_object.row_limit, app.config["ROW_LIMIT"]) self.assertEqual(query_object.row_offset, 0) @@ -49,7 +48,7 @@ def test_query_context_limit_and_offset(self): # Valid limit and offset payload["queries"][0]["row_limit"] = 100 payload["queries"][0]["row_offset"] = 200 - query_context= ChartDataQueryContextSchema().load(payload) + query_context = ChartDataQueryContextSchema().load(payload) query_object = query_context.queries[0] self.assertEqual(query_object.row_limit, 100) self.assertEqual(query_object.row_offset, 200) @@ -58,7 +57,7 @@ def test_query_context_limit_and_offset(self): payload["queries"][0]["row_limit"] = 0 payload["queries"][0]["row_offset"] = -1 try: - query_context = ChartDataQueryContextSchema().load(payload) + _ = ChartDataQueryContextSchema().load(payload) except ValidationError as errors: self.assertIn("row_limit", errors["queries"][0]) self.assertIn("row_offset", errors["queries"][0]) From 600d40421fa9b2987d0d8d5711573ac93fff5f3c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 10:50:50 +0100 Subject: [PATCH 10/17] fix tests --- tests/charts/schema_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/charts/schema_tests.py b/tests/charts/schema_tests.py index eaee3e95c3e3..49d57ed35247 100644 --- a/tests/charts/schema_tests.py +++ b/tests/charts/schema_tests.py @@ -59,8 +59,8 @@ def test_query_context_limit_and_offset(self): try: _ = ChartDataQueryContextSchema().load(payload) except ValidationError as errors: - self.assertIn("row_limit", errors["queries"][0]) - self.assertIn("row_offset", errors["queries"][0]) + self.assertIn("row_limit", errors.messages["queries"][0]) + self.assertIn("row_offset", errors.messages["queries"][0]) def test_query_context_null_timegrain(self): self.login(username="admin") From 126e94299c2ae71299d72320cf52b2db33f167e3 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 11:06:50 +0100 Subject: [PATCH 11/17] bump to RC3 and lint --- requirements.txt | 4 ++-- setup.py | 2 +- superset/charts/api.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements.txt b/requirements.txt index 54b2fb96263a..acb487e76d07 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,7 @@ decorator==4.4.2 # via retry defusedxml==0.6.0 # via python3-openid dnspython==1.16.0 # via email-validator email-validator==1.1.0 # via flask-appbuilder -flask-appbuilder==3.0.0rc2 # via apache-superset (setup.py) +flask-appbuilder==3.0.0rc3 # via apache-superset (setup.py) flask-babel==1.0.0 # via flask-appbuilder flask-caching==1.8.0 # via apache-superset (setup.py) flask-compress==1.5.0 # via apache-superset (setup.py) @@ -100,4 +100,4 @@ yarl==1.4.2 # via aiohttp zipp==3.1.0 # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: -# setuptools \ No newline at end of file +# setuptools diff --git a/setup.py b/setup.py index 66d94ae93c5a..bc2ec7040936 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,7 @@ def get_git_sha(): "cryptography>=2.4.2", "dataclasses<0.7", "flask>=1.1.0, <2.0.0", - "flask-appbuilder==3.0.0rc2", + "flask-appbuilder==3.0.0rc3", "flask-caching", "flask-compress", "flask-talisman", diff --git a/superset/charts/api.py b/superset/charts/api.py index b559d5959e03..eb804f798aca 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -451,7 +451,7 @@ def data(self) -> Response: else: return self.response_400(message="Request is not JSON") try: - query_context = ChartDataQueryContextSchema().load(request.json) + query_context = ChartDataQueryContextSchema().load(json_body) except KeyError: return self.response_400(message="Request is incorrect") except ValidationError as err: From 9c642677e88767f8192cba7e0a57aadcb72e7bdc Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 14:41:39 +0100 Subject: [PATCH 12/17] Test RC4 --- requirements.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index acb487e76d07..e9f2fa1a93bd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,7 @@ decorator==4.4.2 # via retry defusedxml==0.6.0 # via python3-openid dnspython==1.16.0 # via email-validator email-validator==1.1.0 # via flask-appbuilder -flask-appbuilder==3.0.0rc3 # via apache-superset (setup.py) +flask-appbuilder==3.0.0rc4 # via apache-superset (setup.py) flask-babel==1.0.0 # via flask-appbuilder flask-caching==1.8.0 # via apache-superset (setup.py) flask-compress==1.5.0 # via apache-superset (setup.py) diff --git a/setup.py b/setup.py index bc2ec7040936..ad1e110c84dd 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,7 @@ def get_git_sha(): "cryptography>=2.4.2", "dataclasses<0.7", "flask>=1.1.0, <2.0.0", - "flask-appbuilder==3.0.0rc3", + "flask-appbuilder==3.0.0rc4", "flask-caching", "flask-compress", "flask-talisman", From 15d75b4322c5d8af67ec8af811e5c5367b8cf9f1 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 3 Jul 2020 15:02:27 +0100 Subject: [PATCH 13/17] Final 3.0.0 --- requirements.txt | 4 ++-- setup.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements.txt b/requirements.txt index e9f2fa1a93bd..9713105b34d0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ aiohttp==3.6.2 # via slackclient alembic==1.4.2 # via flask-migrate amqp==2.5.2 # via kombu -apispec[yaml]==1.3.3 # via flask-appbuilder +apispec[yaml]==3.3.1 # via flask-appbuilder async-timeout==3.0.1 # via aiohttp attrs==19.3.0 # via aiohttp, jsonschema babel==2.8.0 # via flask-babel @@ -29,7 +29,7 @@ decorator==4.4.2 # via retry defusedxml==0.6.0 # via python3-openid dnspython==1.16.0 # via email-validator email-validator==1.1.0 # via flask-appbuilder -flask-appbuilder==3.0.0rc4 # via apache-superset (setup.py) +flask-appbuilder==3.0.0 # via apache-superset (setup.py) flask-babel==1.0.0 # via flask-appbuilder flask-caching==1.8.0 # via apache-superset (setup.py) flask-compress==1.5.0 # via apache-superset (setup.py) diff --git a/setup.py b/setup.py index ad1e110c84dd..430b68a5754c 100644 --- a/setup.py +++ b/setup.py @@ -78,7 +78,7 @@ def get_git_sha(): "cryptography>=2.4.2", "dataclasses<0.7", "flask>=1.1.0, <2.0.0", - "flask-appbuilder==3.0.0rc4", + "flask-appbuilder>=3.0.0, <4.0.0", "flask-caching", "flask-compress", "flask-talisman", From b16094aaf8bcd7d0ab5df006550623bfd59aa5e7 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 6 Jul 2020 09:52:51 +0100 Subject: [PATCH 14/17] Address comments, fix tests, better naming, docs --- superset/charts/api.py | 12 ++++++------ superset/charts/schemas.py | 3 +-- superset/dashboards/api.py | 8 ++++---- superset/datasets/api.py | 8 ++++---- tests/charts/schema_tests.py | 7 +++---- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/superset/charts/api.py b/superset/charts/api.py index eb804f798aca..2add53624fb9 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -216,8 +216,8 @@ def post(self) -> Response: try: item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: new_model = CreateChartCommand(g.user, item).run() return self.response(201, id=new_model.id, result=item) @@ -283,8 +283,8 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ try: item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: changed_model = UpdateChartCommand(g.user, pk, item).run() return self.response(200, id=changed_model.id, result=item) @@ -454,9 +454,9 @@ def data(self) -> Response: query_context = ChartDataQueryContextSchema().load(json_body) except KeyError: return self.response_400(message="Request is incorrect") - except ValidationError as err: + except ValidationError as error: return self.response_400( - _("Request is incorrect: %(error)s", error=err.messages) + _("Request is incorrect: %(error)s", error=error.messages) ) try: query_context.raise_for_access() diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index d9918c8d6786..9b5950608426 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -664,8 +664,7 @@ class ChartDataQueryObjectSchema(Schema): description="Maximum row count for timeseries queries. Default: `0`", ) timeseries_limit_metric = fields.Integer( - description="Maximum row count for timeseries queries. Default: `0`", - allow_none=True, + description="Metric used to limit timeseries queries by.", allow_none=True, ) row_limit = fields.Integer( description='Maximum row count. Default: `config["ROW_LIMIT"]`', diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index bbcf1b4ae561..ef74d8cf6754 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -201,8 +201,8 @@ def post(self) -> Response: try: item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: new_model = CreateDashboardCommand(g.user, item).run() return self.response(201, id=new_model.id, result=item) @@ -268,8 +268,8 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ try: item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: changed_model = UpdateDashboardCommand(g.user, pk, item).run() return self.response(200, id=changed_model.id, result=item) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index e6a06c7eb663..ab7aa5c32bf0 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -180,8 +180,8 @@ def post(self) -> Response: try: item = self.add_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: new_model = CreateDatasetCommand(g.user, item).run() return self.response(201, id=new_model.id, result=item) @@ -247,8 +247,8 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ try: item = self.edit_model_schema.load(request.json) # This validates custom Schema with custom validations - except ValidationError as err: - return self.response_400(message=err.messages) + except ValidationError as error: + return self.response_400(message=error.messages) try: changed_model = UpdateDatasetCommand(g.user, pk, item).run() return self.response(200, id=changed_model.id, result=item) diff --git a/tests/charts/schema_tests.py b/tests/charts/schema_tests.py index 49d57ed35247..622e5c8f6b44 100644 --- a/tests/charts/schema_tests.py +++ b/tests/charts/schema_tests.py @@ -56,11 +56,10 @@ def test_query_context_limit_and_offset(self): # too low limit and offset payload["queries"][0]["row_limit"] = 0 payload["queries"][0]["row_offset"] = -1 - try: + with self.assertRaises(ValidationError) as errors: _ = ChartDataQueryContextSchema().load(payload) - except ValidationError as errors: - self.assertIn("row_limit", errors.messages["queries"][0]) - self.assertIn("row_offset", errors.messages["queries"][0]) + self.assertIn("row_limit", errors.messages["queries"][0]) + self.assertIn("row_offset", errors.messages["queries"][0]) def test_query_context_null_timegrain(self): self.login(username="admin") From 66a47d00feff333c8f83a67e17f32ce2027e388a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 6 Jul 2020 10:00:27 +0100 Subject: [PATCH 15/17] fix test --- tests/charts/schema_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/charts/schema_tests.py b/tests/charts/schema_tests.py index 622e5c8f6b44..354ed823c4cd 100644 --- a/tests/charts/schema_tests.py +++ b/tests/charts/schema_tests.py @@ -56,10 +56,10 @@ def test_query_context_limit_and_offset(self): # too low limit and offset payload["queries"][0]["row_limit"] = 0 payload["queries"][0]["row_offset"] = -1 - with self.assertRaises(ValidationError) as errors: + with self.assertRaises(ValidationError) as context: _ = ChartDataQueryContextSchema().load(payload) - self.assertIn("row_limit", errors.messages["queries"][0]) - self.assertIn("row_offset", errors.messages["queries"][0]) + self.assertIn("row_limit", context.exception.messages["queries"][0]) + self.assertIn("row_offset", context.exception.messages["queries"][0]) def test_query_context_null_timegrain(self): self.login(username="admin") From eda8b8381c9d1b656415cfc62e26a3afd38e16d4 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 6 Jul 2020 10:36:21 +0100 Subject: [PATCH 16/17] couple of fixes, addressing comments --- superset/charts/schemas.py | 2 +- tests/datasets/api_tests.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 9b5950608426..dd9f1c478bad 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -727,7 +727,7 @@ class ChartDataQueryContextSchema(Schema): ) result_type = fields.String( description="Type of results to return", - validate=validate.OneOf(choices=("query", "results", "samples")), + validate=validate.OneOf(choices=("full", "query", "results", "samples")), ) result_format = fields.String( description="Format of result payload", diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py index d16ff30ab640..49cd2eccbdd6 100644 --- a/tests/datasets/api_tests.py +++ b/tests/datasets/api_tests.py @@ -375,8 +375,6 @@ def test_update_dataset_create_column(self): for column in data["result"]["columns"]: column.pop("changed_on", None) column.pop("created_on", None) - column.pop("changed_on", None) - column.pop("changed_on", None) data["result"]["columns"].append(new_column_data) rv = self.client.put(uri, json={"columns": data["result"]["columns"]}) @@ -414,8 +412,6 @@ def test_update_dataset_update_column(self): for column in resp_columns: column.pop("changed_on", None) column.pop("created_on", None) - column.pop("changed_on", None) - column.pop("changed_on", None) resp_columns[0]["groupby"] = False resp_columns[0]["filterable"] = False From 1c406982f5684d0f9a3c3dc45b387396c7a104f3 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Mon, 6 Jul 2020 11:15:11 +0100 Subject: [PATCH 17/17] bumping marshmallow --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 9713105b34d0..6b2b07198e68 100644 --- a/requirements.txt +++ b/requirements.txt @@ -58,7 +58,7 @@ markdown==3.2.2 # via apache-superset (setup.py) markupsafe==1.1.1 # via jinja2, mako, wtforms marshmallow-enum==1.5.1 # via flask-appbuilder marshmallow-sqlalchemy==0.23.0 # via flask-appbuilder -marshmallow==3.6.0 # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy +marshmallow==3.6.1 # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy msgpack==1.0.0 # via apache-superset (setup.py) multidict==4.7.6 # via aiohttp, yarl numpy==1.18.4 # via pandas, pyarrow