From b4d9b756bede0b1ad4912e1a61533bdb796e787b Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Wed, 25 Jan 2023 16:35:24 +0800 Subject: [PATCH 1/3] Remove unused SUPERSET_WEBSERVER_ config We stopped using SUPERSET_WEBSERVER_PROTOCOL, ..._ADDRESS, ..._PORT with https://github.com/apache/superset/pull/21076. Removing the config from the example config as there does not seem to be any immediate need for it, we already have WEBDRIVER_BASEURL (and the _USER_FRIENDLY variant). --- superset/config.py | 4 ---- tests/integration_tests/superset_test_config.py | 1 - tests/integration_tests/superset_test_config_thumbnails.py | 1 - 3 files changed, 6 deletions(-) diff --git a/superset/config.py b/superset/config.py index 348baef5454af..c250180f1a2ec 100644 --- a/superset/config.py +++ b/superset/config.py @@ -154,10 +154,6 @@ def _try_json_readsha(filepath: str, length: int) -> str | None: # values may be "Last day", "Last week", " : now", etc. DEFAULT_TIME_FILTER = NO_TIME_RANGE -SUPERSET_WEBSERVER_PROTOCOL = "http" -SUPERSET_WEBSERVER_ADDRESS = "0.0.0.0" -SUPERSET_WEBSERVER_PORT = 8088 - # This is an important setting, and should be lower than your # [load balancer / proxy / envoy / kong / ...] timeout settings. # You should also make sure to configure your WSGI server diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index 89287be663e55..667d337017a85 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -36,7 +36,6 @@ DATA_DIR, "unittests.integration_tests.db" ) DEBUG = False -SUPERSET_WEBSERVER_PORT = 8081 SILENCE_FAB = False # Allowing SQLALCHEMY_DATABASE_URI and SQLALCHEMY_EXAMPLES_URI to be defined as an env vars for # continuous integration diff --git a/tests/integration_tests/superset_test_config_thumbnails.py b/tests/integration_tests/superset_test_config_thumbnails.py index a761ef6611bb6..0b5710d3424d5 100644 --- a/tests/integration_tests/superset_test_config_thumbnails.py +++ b/tests/integration_tests/superset_test_config_thumbnails.py @@ -24,7 +24,6 @@ DATA_DIR, "unittests.integration_tests.db" ) DEBUG = True -SUPERSET_WEBSERVER_PORT = 8081 # Allowing SQLALCHEMY_DATABASE_URI to be defined as an env var for # continuous integration From 5a5b113f0a07d331643ac035cbe259c3e7dc6097 Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Thu, 27 Apr 2023 17:02:37 +0200 Subject: [PATCH 2/3] Handle WEBDRIVER_BASEURL slashes As suggested, we handle both cases (trailing slash, no trailing slash) for the WEBDRIVER_BASEURL. We use the existing `superset.utils.urls` `get_url_path(...)` function to manage everything. --- superset/tasks/cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index 569797ba27ab0..1f60a5cd84451 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -32,6 +32,7 @@ from superset.tags.models import Tag, TaggedObject from superset.utils.date_parser import parse_human_datetime from superset.utils.machine_auth import MachineAuthProvider +from superset.utils.urls import get_url_path logger = get_task_logger(__name__) logger.setLevel(logging.INFO) @@ -233,8 +234,7 @@ def fetch_url(data: str, headers: dict[str, str]) -> dict[str, str]: """ result = {} try: - baseurl = app.config["WEBDRIVER_BASEURL"] - url = f"{baseurl}api/v1/chart/warm_up_cache" + url = get_url_path("Superset.warm_up_cache") logger.info("Fetching %s with payload %s", url, data) req = request.Request( url, data=bytes(data, "utf-8"), headers=headers, method="PUT" From b8acc6be1932db4491ccc597515eb29637900ba9 Mon Sep 17 00:00:00 2001 From: Usiel Riedl Date: Mon, 27 Nov 2023 16:04:28 +0800 Subject: [PATCH 3/3] Adds test to ensure trailing slashes are handled for fetch_url task --- tests/integration_tests/tasks/test_cache.py | 58 +++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/integration_tests/tasks/test_cache.py diff --git a/tests/integration_tests/tasks/test_cache.py b/tests/integration_tests/tasks/test_cache.py new file mode 100644 index 0000000000000..943b444f76936 --- /dev/null +++ b/tests/integration_tests/tasks/test_cache.py @@ -0,0 +1,58 @@ +# 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. +from unittest import mock + +import pytest + +from tests.integration_tests.test_app import app + + +@pytest.mark.parametrize( + "base_url", + [ + "http://base-url", + "http://base-url/", + ], + ids=["Without trailing slash", "With trailing slash"], +) +@mock.patch("superset.tasks.cache.request.Request") +@mock.patch("superset.tasks.cache.request.urlopen") +def test_fetch_url(mock_urlopen, mock_request_cls, base_url): + from superset.tasks.cache import fetch_url + + mock_request = mock.MagicMock() + mock_request_cls.return_value = mock_request + + mock_urlopen.return_value = mock.MagicMock() + mock_urlopen.return_value.code = 200 + + app.config["WEBDRIVER_BASEURL"] = base_url + headers = {"key": "value"} + data = "data" + data_encoded = b"data" + + result = fetch_url(data, headers) + + assert data == result["success"] + mock_request_cls.assert_called_once_with( + "http://base-url/superset/warm_up_cache/", + data=data_encoded, + headers=headers, + method="PUT", + ) + # assert the same Request object is used + mock_urlopen.assert_called_once_with(mock_request, timeout=mock.ANY)