You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2023/10/20 22:42:00 UTC
[superset] branch 2.1 updated (62547ccd05 -> 79d0cd5389)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a change to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from 62547ccd05 fix: update order of build for testing a release (#24317)
add 3d840d92e6 fix: Allow chart import to update the dataset an existing chart points to (#24821)
add 2a49cda8f1 fix: SSH Tunnel creation with dynamic form (#24196)
add 61837ed41f fix: validation errors appearing after ssh tunnel switch (#24849)
new b55c950c37 chore: remove CssTemplate and Annotation access from gamma role (#24826)
new 1b534292bc fix: CTE queries with non-SELECT statements (#25014)
new bf795f9515 fix: Chart series limit doesn't work for some databases (#25150)
new 79d0cd5389 chore: add latest-official docker tag (#25322)
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.github/workflows/docker-release.yml | 3 +-
.github/workflows/docker.yml | 2 +-
.../workflows => scripts}/docker_build_push.sh | 82 +++++++-
scripts/tag_latest_release.sh | 140 ++++++++++----
.../cypress/integration/database/modal.test.ts | 8 +-
superset-frontend/src/types/Database.ts | 1 +
.../DatabaseConnectionForm/CommonParameters.tsx | 34 ++++
.../DatabaseModal/DatabaseConnectionForm/index.tsx | 7 +
.../data/database/DatabaseModal/index.test.tsx | 4 +
.../CRUD/data/database/DatabaseModal/index.tsx | 32 ++--
.../src/views/CRUD/data/database/types.ts | 1 +
superset-frontend/src/views/CRUD/hooks.ts | 209 +++++++++++----------
superset/charts/commands/importers/v1/utils.py | 5 +-
superset/db_engine_specs/base.py | 4 +
superset/models/helpers.py | 56 +++++-
superset/security/manager.py | 22 +--
superset/sql_parse.py | 55 ++++++
tests/integration_tests/databases/api_tests.py | 12 ++
.../db_engine_specs/postgres_tests.py | 4 +
tests/integration_tests/security_tests.py | 3 -
.../unit_tests/fixtures/bash_mock.py | 48 ++---
tests/unit_tests/scripts/docker_build_push_test.py | 44 +++++
.../unit_tests/scripts/tag_latest_release_test.py | 49 +++++
tests/unit_tests/sql_parse_tests.py | 103 ++++++++++
24 files changed, 719 insertions(+), 209 deletions(-)
rename {.github/workflows => scripts}/docker_build_push.sh (57%)
copy superset/migrations/versions/2017-09-19_15-09_d39b1e37131d_.py => tests/unit_tests/fixtures/bash_mock.py (53%)
create mode 100644 tests/unit_tests/scripts/docker_build_push_test.py
create mode 100644 tests/unit_tests/scripts/tag_latest_release_test.py
[superset] 03/04: fix: Chart series limit doesn't work for some databases (#25150)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bf795f9515e955c770af12f8734700607a902a13
Author: KSPT-taylorjohn <52...@users.noreply.github.com>
AuthorDate: Thu Aug 31 18:05:39 2023 -0400
fix: Chart series limit doesn't work for some databases (#25150)
---
superset/models/helpers.py | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 59db27d404..8e9e8e4302 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -100,6 +100,7 @@ config = app.config
logger = logging.getLogger(__name__)
VIRTUAL_TABLE_ALIAS = "virtual_table"
+SERIES_LIMIT_SUBQ_ALIAS = "series_limit"
ADVANCED_DATA_TYPES = config["ADVANCED_DATA_TYPES"]
@@ -1391,7 +1392,19 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
}
columns = columns or []
groupby = groupby or []
+<<<<<<< HEAD
series_column_names = utils.get_column_names(series_columns or [])
+=======
+ rejected_adhoc_filters_columns: list[Union[str, ColumnTyping]] = []
+ applied_adhoc_filters_columns: list[Union[str, ColumnTyping]] = []
+ db_engine_spec = self.db_engine_spec
+ series_column_labels = [
+ db_engine_spec.make_label_compatible(column)
+ for column in utils.get_column_names(
+ columns=series_columns or [],
+ )
+ ]
+>>>>>>> bbfaeb074... fix: Chart series limit doesn't work for some databases (#25150)
# deprecated, to be removed in 2.0
if is_timeseries and timeseries_limit:
series_limit = timeseries_limit
@@ -1404,8 +1417,12 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
template_kwargs["removed_filters"] = removed_filters
template_kwargs["applied_filters"] = applied_template_filters
template_processor = self.get_template_processor(**template_kwargs)
+<<<<<<< HEAD
db_engine_spec = self.db_engine_spec
prequeries: List[str] = []
+=======
+ prequeries: list[str] = []
+>>>>>>> bbfaeb074... fix: Chart series limit doesn't work for some databases (#25150)
orderby = orderby or []
need_groupby = bool(metrics is not None or groupby)
metrics = metrics or []
@@ -1545,8 +1562,8 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
)
groupby_all_columns[outer.name] = outer
if (
- is_timeseries and not series_column_names
- ) or outer.name in series_column_names:
+ is_timeseries and not series_column_labels
+ ) or outer.name in series_column_labels:
groupby_series_columns[outer.name] = outer
select_exprs.append(outer)
elif columns:
@@ -1926,7 +1943,24 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
col_name = db_engine_spec.make_label_compatible(gby_name + "__")
on_clause.append(gby_obj == sa.column(col_name))
+<<<<<<< HEAD
tbl = tbl.join(subq.alias(), and_(*on_clause))
+=======
+ tbl = tbl.join(subq.alias(SERIES_LIMIT_SUBQ_ALIAS), and_(*on_clause))
+ else:
+ if series_limit_metric:
+ orderby = [
+ (
+ self._get_series_orderby(
+ series_limit_metric=series_limit_metric,
+ metrics_by_name=metrics_by_name,
+ columns_by_name=columns_by_name,
+ template_processor=template_processor,
+ ),
+ not order_desc,
+ )
+ ]
+>>>>>>> bbfaeb074... fix: Chart series limit doesn't work for some databases (#25150)
# run prequery to get top groups
prequery_obj = {
[superset] 01/04: chore: remove CssTemplate and Annotation access from gamma role (#24826)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit b55c950c37fee34af33eac48eab042f069de0d8e
Author: Lily Kuang <li...@preset.io>
AuthorDate: Thu Aug 24 16:39:56 2023 -0700
chore: remove CssTemplate and Annotation access from gamma role (#24826)
---
superset/security/manager.py | 22 +++++++++++-----------
tests/integration_tests/security_tests.py | 3 ---
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 6f3a4b90bf..391704c41b 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -167,8 +167,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
}
GAMMA_READ_ONLY_MODEL_VIEWS = {
- "Annotation",
- "CssTemplate",
"Dataset",
"Datasource",
} | READ_ONLY_MODEL_VIEWS
@@ -191,19 +189,21 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
} | USER_MODEL_VIEWS
ALPHA_ONLY_VIEW_MENUS = {
- "Manage",
- "CSS Templates",
- "Annotation Layers",
- "Queries",
- "Import dashboards",
- "Upload a CSV",
- "ReportSchedule",
"Alerts & Report",
- "TableSchemaView",
- "CsvToDatabaseView",
+ "Annotation Layers",
+ "Annotation",
+ "CSS Templates",
"ColumnarToDatabaseView",
+ "CssTemplate",
+ "CsvToDatabaseView",
"ExcelToDatabaseView",
+ "Import dashboards",
"ImportExportRestApi",
+ "Manage",
+ "Queries",
+ "ReportSchedule",
+ "TableSchemaView",
+ "Upload a CSV",
}
ADMIN_ONLY_PERMISSIONS = {
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index c65f5a6dd8..55fc1f2dad 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -1345,7 +1345,6 @@ class TestRolePermission(SupersetTestCase):
self.assert_cannot_menu("Alerts & Report", perm_set)
def assert_can_gamma(self, perm_set):
- self.assert_can_read("CssTemplate", perm_set)
self.assert_can_read("Dataset", perm_set)
# make sure that user can create slices and dashboards
@@ -1552,8 +1551,6 @@ class TestRolePermission(SupersetTestCase):
# make sure that user can create slices and dashboards
self.assert_can_all("Dashboard", gamma_perm_set)
self.assert_can_read("Dataset", gamma_perm_set)
- self.assert_can_read("Annotation", gamma_perm_set)
- self.assert_can_read("CssTemplate", gamma_perm_set)
# make sure that user can create slices and dashboards
self.assert_can_all("Chart", gamma_perm_set)
[superset] 04/04: chore: add latest-official docker tag (#25322)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 79d0cd5389275cf57ab424aeba0b46df91bf6c49
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Wed Oct 18 16:59:30 2023 -0700
chore: add latest-official docker tag (#25322)
---
.github/workflows/docker-release.yml | 3 +-
.github/workflows/docker.yml | 2 +-
.../workflows => scripts}/docker_build_push.sh | 82 +++++++++++-
scripts/tag_latest_release.sh | 140 +++++++++++++++------
tests/unit_tests/fixtures/bash_mock.py | 44 +++++++
tests/unit_tests/scripts/docker_build_push_test.py | 44 +++++++
.../unit_tests/scripts/tag_latest_release_test.py | 49 ++++++++
7 files changed, 322 insertions(+), 42 deletions(-)
diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml
index d082603be9..9e9119ab78 100644
--- a/.github/workflows/docker-release.yml
+++ b/.github/workflows/docker-release.yml
@@ -19,4 +19,5 @@ jobs:
DOCKERHUB_USER: ${{ secrets.DOCKERHUB_USER }}
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
run: |
- .github/workflows/docker_build_push.sh
+ GITHUB_RELEASE_TAG_NAME="${{ github.event.release.tag_name }}"
+ ./scripts/docker_build_push.sh "$GITHUB_RELEASE_TAG_NAME"
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
index bd3fa3730c..8e62938baf 100644
--- a/.github/workflows/docker.yml
+++ b/.github/workflows/docker.yml
@@ -41,7 +41,7 @@ jobs:
DOCKERHUB_USER: ${{ secrets.DOCKERHUB_USER }}
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
run: |
- .github/workflows/docker_build_push.sh
+ ./scripts/docker_build_push.sh
- name: Build ephemeral env image
if: github.event_name == 'pull_request'
diff --git a/.github/workflows/docker_build_push.sh b/scripts/docker_build_push.sh
similarity index 57%
rename from .github/workflows/docker_build_push.sh
rename to scripts/docker_build_push.sh
index b969813627..4aef8f2244 100755
--- a/.github/workflows/docker_build_push.sh
+++ b/scripts/docker_build_push.sh
@@ -17,6 +17,8 @@
#
set -eo pipefail
+GITHUB_RELEASE_TAG_NAME="$1"
+
SHA=$(git rev-parse HEAD)
REPO_NAME="apache/superset"
@@ -32,10 +34,27 @@ else
LATEST_TAG="${REFSPEC}"
fi
+
if [[ "${REFSPEC}" == "master" ]]; then
- LATEST_TAG="latest"
+ LATEST_TAG="master"
+fi
+
+# get the latest release tag
+if [ -n "${GITHUB_RELEASE_TAG_NAME}" ]; then
+ output=$(source ./scripts/tag_latest_release.sh "${GITHUB_RELEASE_TAG_NAME}" --dry-run) || true
+ SKIP_TAG=$(echo "${output}" | grep "SKIP_TAG" | cut -d'=' -f2)
+ if [[ "${SKIP_TAG}" == "SKIP_TAG::false" ]]; then
+ LATEST_TAG="latest"
+ fi
+fi
+
+if [[ "${TEST_ENV}" == "true" ]]; then
+ # don't run the build in test environment
+ echo "LATEST_TAG is ${LATEST_TAG}"
+ exit 0
fi
+
cat<<EOF
Rolling with tags:
- ${REPO_NAME}:${SHA}
@@ -43,6 +62,47 @@ cat<<EOF
- ${REPO_NAME}:${LATEST_TAG}
EOF
+if [ -z "${DOCKERHUB_TOKEN}" ]; then
+ # Skip if secrets aren't populated -- they're only visible for actions running in the repo (not on forks)
+ echo "Skipping Docker push"
+ # By default load it back
+ DOCKER_ARGS="--load"
+ ARCHITECTURE_FOR_BUILD="linux/amd64 linux/arm64"
+else
+ # Login and push
+ docker logout
+ docker login --username "${DOCKERHUB_USER}" --password "${DOCKERHUB_TOKEN}"
+ DOCKER_ARGS="--push"
+ ARCHITECTURE_FOR_BUILD="linux/amd64,linux/arm64"
+fi
+set -x
+
+# for the dev image, it's ok to tag master as latest-dev
+# for production, we only want to tag the latest official release as latest
+if [ "${LATEST_TAG}" = "master" ]; then
+ DEV_TAG="${REPO_NAME}:latest-dev"
+else
+ DEV_TAG="${REPO_NAME}:${LATEST_TAG}-dev"
+fi
+
+#
+# Build the dev image
+#
+docker buildx build --target dev \
+ $DOCKER_ARGS \
+ --cache-from=type=registry,ref=apache/superset:master-dev \
+ --cache-from=type=local,src=/tmp/superset \
+ --cache-to=type=local,ignore-error=true,dest=/tmp/superset \
+ -t "${REPO_NAME}:${SHA}-dev" \
+ -t "${REPO_NAME}:${REFSPEC}-dev" \
+ -t "${DEV_TAG}" \
+ --platform linux/amd64 \
+ --label "sha=${SHA}" \
+ --label "built_at=$(date)" \
+ --label "target=dev" \
+ --label "build_actor=${GITHUB_ACTOR}" \
+ .
+
#
# Build the "lean" image
#
@@ -71,6 +131,26 @@ docker build --target lean \
--label "build_actor=${GITHUB_ACTOR}" \
.
+#
+# Build the "lean39" image
+#
+docker buildx build --target lean \
+ $DOCKER_ARGS \
+ --cache-from=type=local,src=/tmp/superset \
+ --cache-to=type=local,ignore-error=true,dest=/tmp/superset \
+ -t "${REPO_NAME}:${SHA}-py39" \
+ -t "${REPO_NAME}:${REFSPEC}-py39" \
+ -t "${REPO_NAME}:${LATEST_TAG}-py39" \
+ --platform linux/amd64 \
+ --build-arg PY_VER="3.9-slim-bullseye"\
+ --label "sha=${SHA}" \
+ --label "built_at=$(date)" \
+ --label "target=lean39" \
+ --label "build_actor=${GITHUB_ACTOR}" \
+ .
+
+
+for BUILD_PLATFORM in $ARCHITECTURE_FOR_BUILD; do
#
# Build the "websocket" image
#
diff --git a/scripts/tag_latest_release.sh b/scripts/tag_latest_release.sh
index a5d50eaf9f..f2ee846a63 100755
--- a/scripts/tag_latest_release.sh
+++ b/scripts/tag_latest_release.sh
@@ -17,7 +17,7 @@
#
run_git_tag () {
- if [ "$DRY_RUN" = "false" ] && [ "$SKIP_TAG" = "false" ]
+ if [[ "$DRY_RUN" == "false" ]] && [[ "$SKIP_TAG" == "false" ]]
then
git tag -a -f latest "${GITHUB_TAG_NAME}" -m "latest tag"
echo "${GITHUB_TAG_NAME} has been tagged 'latest'"
@@ -25,7 +25,55 @@ run_git_tag () {
exit 0
}
-echo "::set-output name=SKIP_TAG::false"
+###
+# separating out git commands into functions so they can be mocked in unit tests
+###
+git_show_ref () {
+ if [[ "$TEST_ENV" == "true" ]]
+ then
+ if [[ "$GITHUB_TAG_NAME" == "does_not_exist" ]]
+ # mock return for testing only
+ then
+ echo ""
+ else
+ echo "2817aebd69dc7d199ec45d973a2079f35e5658b6 refs/tags/${GITHUB_TAG_NAME}"
+ fi
+ fi
+ result=$(git show-ref "${GITHUB_TAG_NAME}")
+ echo "${result}"
+}
+
+get_latest_tag_list () {
+ if [[ "$TEST_ENV" == "true" ]]
+ then
+ echo "(tag: 2.1.0, apache/2.1test)"
+ else
+ result=$(git show-ref --tags --dereference latest | awk '{print $2}' | xargs git show --pretty=tformat:%d -s | grep tag:)
+ echo "${result}"
+ fi
+}
+###
+
+split_string () {
+ local version="$1"
+ local delimiter="$2"
+ local components=()
+ local tmp=""
+ for (( i=0; i<${#version}; i++ )); do
+ local char="${version:$i:1}"
+ if [[ "$char" != "$delimiter" ]]; then
+ tmp="$tmp$char"
+ elif [[ -n "$tmp" ]]; then
+ components+=("$tmp")
+ tmp=""
+ fi
+ done
+ if [[ -n "$tmp" ]]; then
+ components+=("$tmp")
+ fi
+ echo "${components[@]}"
+}
+
DRY_RUN=false
# get params passed in with script when it was run
@@ -50,12 +98,14 @@ done
if [ -z "${GITHUB_TAG_NAME}" ]; then
echo "Missing tag parameter, usage: ./scripts/tag_latest_release.sh <GITHUB_TAG_NAME>"
+ echo "::set-output name=SKIP_TAG::true"
exit 1
fi
-if [ -z "$(git show-ref ${GITHUB_TAG_NAME})" ]; then
+if [ -z "$(git_show_ref)" ]; then
echo "The tag ${GITHUB_TAG_NAME} does not exist. Please use a different tag."
- exit 1
+ echo "::set-output name=SKIP_TAG::true"
+ exit 0
fi
# check that this tag only contains a proper semantic version
@@ -63,36 +113,41 @@ if ! [[ ${GITHUB_TAG_NAME} =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]
then
echo "This tag ${GITHUB_TAG_NAME} is not a valid release version. Not tagging."
echo "::set-output name=SKIP_TAG::true"
- exit 0
+ exit 1
fi
## split the current GITHUB_TAG_NAME into an array at the dot
-IFS=$'.'
-THIS_TAG_NAME=(${GITHUB_TAG_NAME}) || echo 'not found'
+THIS_TAG_NAME=$(split_string "${GITHUB_TAG_NAME}" ".")
# look up the 'latest' tag on git
-LATEST_TAG_LIST=$(git show-ref latest && git show --pretty=tformat:%d -s latest | grep tag:) || echo 'not found'
+LATEST_TAG_LIST=$(get_latest_tag_list) || echo 'not found'
# if 'latest' tag doesn't exist, then set this commit to latest
if [[ -z "$LATEST_TAG_LIST" ]]
then
- # move on to next task
echo "there are no latest tags yet, so I'm going to start by tagging this sha as the latest"
run_git_tag
+ exit 0
fi
-## get all tags that use the same sha as the latest tag. split at comma.
-IFS=$','
-LATEST_TAGS=($LATEST_TAG_LIST)
+# remove parenthesis and tag: from the list of tags
+LATEST_TAGS_STRINGS=$(echo "$LATEST_TAG_LIST" | sed 's/tag: \([^,]*\)/\1/g' | tr -d '()')
-## loop over those tags and only take action on the one that isn't tagged 'latest'
-## that one will have the version number tag
-for (( i=0; i<${#LATEST_TAGS[@]}; i++ ))
+LATEST_TAGS=$(split_string "$LATEST_TAGS_STRINGS" ",")
+TAGS=($(split_string "$LATEST_TAGS" " "))
+
+# Initialize a flag for comparison result
+compare_result=""
+
+# Iterate through the tags of the latest release
+for tag in $TAGS
do
- if [[ ${LATEST_TAGS[$i]} != *"latest"* ]]
- then
+ if [[ $tag == "latest" ]]; then
+ continue
+ else
## extract just the version from this tag
- LATEST_RELEASE_TAG=$(echo "${LATEST_TAGS[$i]}" | sed -E -e 's/tag:|\(|\)|[[:space:]]*//g')
+ LATEST_RELEASE_TAG="$tag"
+ echo "LATEST_RELEASE_TAG: ${LATEST_RELEASE_TAG}"
# check that this only contains a proper semantic version
if ! [[ ${LATEST_RELEASE_TAG} =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]
@@ -101,28 +156,35 @@ do
continue
fi
echo "The current release with the latest tag is version ${LATEST_RELEASE_TAG}"
-
- ## remove the sha from the latest tag and split into an array- split at the dot
- IFS=$'.'
- LATEST_RELEASE_TAG_SPLIT=(${LATEST_RELEASE_TAG})
-
- for (( j=0; j<${#THIS_TAG_NAME[@]}; j++ ))
- do
- ## if this value is greater than the latest release, then tag it, if it's lower, then stop, if it's
- ## the same then move on to the next index
- if [[ ${THIS_TAG_NAME[$j]} -gt ${LATEST_RELEASE_TAG_SPLIT[$j]} ]]
- then
- echo "This release tag ${GITHUB_TAG_NAME} is the latest. Tagging it"
- run_git_tag
-
- elif [[ ${THIS_TAG_NAME[$j]} -lt ${LATEST_RELEASE_TAG_SPLIT[$j]} ]]
- then
- continue
- fi
+ # Split the version strings into arrays
+ THIS_TAG_NAME_ARRAY=($(split_string "$THIS_TAG_NAME" "."))
+ LATEST_RELEASE_TAG_ARRAY=($(split_string "$LATEST_RELEASE_TAG" "."))
+
+ # Iterate through the components of the version strings
+ for (( j=0; j<${#THIS_TAG_NAME_ARRAY[@]}; j++ )); do
+ echo "Comparing ${THIS_TAG_NAME_ARRAY[$j]} to ${LATEST_RELEASE_TAG_ARRAY[$j]}"
+ if [[ $((THIS_TAG_NAME_ARRAY[$j])) > $((LATEST_RELEASE_TAG_ARRAY[$j])) ]]; then
+ compare_result="greater"
+ break
+ elif [[ $((THIS_TAG_NAME_ARRAY[$j])) < $((LATEST_RELEASE_TAG_ARRAY[$j])) ]]; then
+ compare_result="lesser"
+ break
+ fi
done
fi
done
-echo "This release tag ${GITHUB_TAG_NAME} is not the latest. Not tagging."
-# if you've gotten this far, then we don't want to run any tags in the next step
-echo "::set-output name=SKIP_TAG::true"
+# Determine the result based on the comparison
+if [[ -z "$compare_result" ]]; then
+ echo "Versions are equal"
+ echo "::set-output name=SKIP_TAG::true"
+elif [[ "$compare_result" == "greater" ]]; then
+ echo "This release tag ${GITHUB_TAG_NAME} is newer than the latest."
+ echo "::set-output name=SKIP_TAG::false"
+ # Add other actions you want to perform for a newer version
+elif [[ "$compare_result" == "lesser" ]]; then
+ echo "This release tag ${GITHUB_TAG_NAME} is older than the latest."
+ echo "This release tag ${GITHUB_TAG_NAME} is not the latest. Not tagging."
+ # if you've gotten this far, then we don't want to run any tags in the next step
+ echo "::set-output name=SKIP_TAG::true"
+fi
diff --git a/tests/unit_tests/fixtures/bash_mock.py b/tests/unit_tests/fixtures/bash_mock.py
new file mode 100644
index 0000000000..91194de6bf
--- /dev/null
+++ b/tests/unit_tests/fixtures/bash_mock.py
@@ -0,0 +1,44 @@
+# 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 subprocess
+
+
+class BashMock:
+ @staticmethod
+ def tag_latest_release(tag):
+ bash_command = f"./scripts/tag_latest_release.sh {tag} --dry-run"
+ result = subprocess.run(
+ bash_command,
+ shell=True,
+ capture_output=True,
+ text=True,
+ env={"TEST_ENV": "true"},
+ )
+ return result
+
+ @staticmethod
+ def docker_build_push(tag, branch):
+ bash_command = f"./scripts/docker_build_push.sh {tag}"
+ result = subprocess.run(
+ bash_command,
+ shell=True,
+ capture_output=True,
+ text=True,
+ env={"TEST_ENV": "true", "GITHUB_REF": f"refs/heads/{branch}"},
+ )
+ return result
diff --git a/tests/unit_tests/scripts/docker_build_push_test.py b/tests/unit_tests/scripts/docker_build_push_test.py
new file mode 100644
index 0000000000..9f4c84318e
--- /dev/null
+++ b/tests/unit_tests/scripts/docker_build_push_test.py
@@ -0,0 +1,44 @@
+import re
+import subprocess
+from unittest import mock
+from unittest.mock import patch
+
+import pytest
+
+from tests.unit_tests.fixtures.bash_mock import BashMock
+
+original_run = subprocess.run
+
+
+def wrapped(*args, **kwargs):
+ return original_run(*args, **kwargs)
+
+
+@pytest.mark.parametrize(
+ "tag, expected_output, branch",
+ [
+ ("1.0.0", "LATEST_TAG is master", "master"),
+ ("2.1.0", "LATEST_TAG is master", "master"),
+ ("2.1.1", "LATEST_TAG is latest", "master"),
+ ("3.0.0", "LATEST_TAG is latest", "master"),
+ ("2.1.0rc1", "LATEST_TAG is 2.1.0", "2.1.0"),
+ ("", "LATEST_TAG is foo", "foo"),
+ ("2.1", "LATEST_TAG is 2.1", "2.1"),
+ ("does_not_exist", "LATEST_TAG is does-not-exist", "does_not_exist"),
+ ],
+)
+def test_tag_latest_release(tag, expected_output, branch):
+ with mock.patch(
+ "tests.unit_tests.fixtures.bash_mock.subprocess.run", wraps=wrapped
+ ) as subprocess_mock:
+ result = BashMock.docker_build_push(tag, branch)
+
+ subprocess_mock.assert_called_once_with(
+ f"./scripts/docker_build_push.sh {tag}",
+ shell=True,
+ capture_output=True,
+ text=True,
+ env={"TEST_ENV": "true", "GITHUB_REF": f"refs/heads/{branch}"},
+ )
+
+ assert re.search(expected_output, result.stdout, re.MULTILINE)
diff --git a/tests/unit_tests/scripts/tag_latest_release_test.py b/tests/unit_tests/scripts/tag_latest_release_test.py
new file mode 100644
index 0000000000..7b15a1670a
--- /dev/null
+++ b/tests/unit_tests/scripts/tag_latest_release_test.py
@@ -0,0 +1,49 @@
+import subprocess
+from unittest import mock
+from unittest.mock import patch
+
+import pytest
+
+from tests.unit_tests.fixtures.bash_mock import BashMock
+
+original_run = subprocess.run
+
+
+def wrapped(*args, **kwargs):
+ return original_run(*args, **kwargs)
+
+
+@pytest.mark.parametrize(
+ "tag, expected_output",
+ [
+ ("1.0.0", "This release tag 1.0.0 is older than the latest."),
+ ("2.1.0", "Versions are equal\n::set-output name=SKIP_TAG::true"),
+ ("2.1.1", "This release tag 2.1.1 is newer than the latest."),
+ ("3.0.0", "This release tag 3.0.0 is newer than the latest."),
+ ("2.1.0rc1", "This tag 2.1.0rc1 is not a valid release version. Not tagging."),
+ (
+ "",
+ "Missing tag parameter, usage: ./scripts/tag_latest_release.sh <GITHUB_TAG_NAME>",
+ ),
+ ("2.1", "This tag 2.1 is not a valid release version. Not tagging."),
+ (
+ "does_not_exist",
+ "The tag does_not_exist does not exist. Please use a different tag.\n::set-output name=SKIP_TAG::true",
+ ),
+ ],
+)
+def test_tag_latest_release(tag, expected_output):
+ with mock.patch(
+ "tests.unit_tests.fixtures.bash_mock.subprocess.run", wraps=wrapped
+ ) as subprocess_mock:
+ result = BashMock.tag_latest_release(tag)
+
+ subprocess_mock.assert_called_once_with(
+ f"./scripts/tag_latest_release.sh {tag} --dry-run",
+ shell=True,
+ capture_output=True,
+ text=True,
+ env={"TEST_ENV": "true"},
+ )
+
+ assert expected_output in result.stdout
[superset] 02/04: fix: CTE queries with non-SELECT statements (#25014)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1b534292bc32ccddfcd74be83cbb25ed0bd5f40f
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Sat Aug 19 15:49:15 2023 +0100
fix: CTE queries with non-SELECT statements (#25014)
---
superset/sql_parse.py | 55 +++++++++++++++++++
tests/unit_tests/sql_parse_tests.py | 103 ++++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
diff --git a/superset/sql_parse.py b/superset/sql_parse.py
index a3c1af87b0..216b4e8825 100644
--- a/superset/sql_parse.py
+++ b/superset/sql_parse.py
@@ -216,9 +216,53 @@ class ParsedQuery:
def limit(self) -> Optional[int]:
return self._limit
+ def _get_cte_tables(self, parsed: dict[str, Any]) -> list[dict[str, Any]]:
+ if "with" not in parsed:
+ return []
+ return parsed["with"].get("cte_tables", [])
+
+ def _check_cte_is_select(self, oxide_parse: list[dict[str, Any]]) -> bool:
+ """
+ Check if a oxide parsed CTE contains only SELECT statements
+
+ :param oxide_parse: parsed CTE
+ :return: True if CTE is a SELECT statement
+ """
+ for query in oxide_parse:
+ parsed_query = query["Query"]
+ cte_tables = self._get_cte_tables(parsed_query)
+ for cte_table in cte_tables:
+ is_select = all(
+ key == "Select" for key in cte_table["query"]["body"].keys()
+ )
+ if not is_select:
+ return False
+ return True
+
def is_select(self) -> bool:
# make sure we strip comments; prevents a bug with coments in the CTE
parsed = sqlparse.parse(self.strip_comments())
+
+ # Check if this is a CTE
+ if parsed[0].is_group and parsed[0][0].ttype == Keyword.CTE:
+ if sqloxide_parse is not None:
+ try:
+ if not self._check_cte_is_select(
+ sqloxide_parse(self.strip_comments(), dialect="ansi")
+ ):
+ return False
+ except ValueError:
+ # sqloxide was not able to parse the query, so let's continue with
+ # sqlparse
+ pass
+ inner_cte = self.get_inner_cte_expression(parsed[0].tokens) or []
+ # Check if the inner CTE is a not a SELECT
+ if any(token.ttype == DDL for token in inner_cte) or any(
+ token.ttype == DML and token.normalized != "SELECT"
+ for token in inner_cte
+ ):
+ return False
+
if parsed[0].get_type() == "SELECT":
return True
@@ -240,6 +284,17 @@ class ParsedQuery:
token.ttype == DML and token.value == "SELECT" for token in parsed[0]
)
+ def get_inner_cte_expression(self, tokens: TokenList) -> Optional[TokenList]:
+ for token in tokens:
+ if self._is_identifier(token):
+ for identifier_token in token.tokens:
+ if (
+ isinstance(identifier_token, Parenthesis)
+ and identifier_token.is_group
+ ):
+ return identifier_token.tokens
+ return None
+
def is_valid_ctas(self) -> bool:
parsed = sqlparse.parse(self.strip_comments())
return parsed[-1].get_type() == "SELECT"
diff --git a/tests/unit_tests/sql_parse_tests.py b/tests/unit_tests/sql_parse_tests.py
index d6939fa080..09eeabce2f 100644
--- a/tests/unit_tests/sql_parse_tests.py
+++ b/tests/unit_tests/sql_parse_tests.py
@@ -1008,6 +1008,109 @@ FROM foo f"""
assert sql.is_select()
+def test_cte_is_select_lowercase() -> None:
+ """
+ Some CTEs with lowercase select are not correctly identified as SELECTS.
+ """
+ sql = ParsedQuery(
+ """WITH foo AS(
+select
+ FLOOR(__time TO WEEK) AS "week",
+ name,
+ COUNT(DISTINCT user_id) AS "unique_users"
+FROM "druid"."my_table"
+GROUP BY 1,2
+)
+select
+ f.week,
+ f.name,
+ f.unique_users
+FROM foo f"""
+ )
+ assert sql.is_select()
+
+
+def test_cte_insert_is_not_select() -> None:
+ """
+ Some CTEs with lowercase select are not correctly identified as SELECTS.
+ """
+ sql = ParsedQuery(
+ """WITH foo AS(
+ INSERT INTO foo (id) VALUES (1) RETURNING 1
+ ) select * FROM foo f"""
+ )
+ assert sql.is_select() is False
+
+
+def test_cte_delete_is_not_select() -> None:
+ """
+ Some CTEs with lowercase select are not correctly identified as SELECTS.
+ """
+ sql = ParsedQuery(
+ """WITH foo AS(
+ DELETE FROM foo RETURNING *
+ ) select * FROM foo f"""
+ )
+ assert sql.is_select() is False
+
+
+def test_cte_is_not_select_lowercase() -> None:
+ """
+ Some CTEs with lowercase select are not correctly identified as SELECTS.
+ """
+ sql = ParsedQuery(
+ """WITH foo AS(
+ insert into foo (id) values (1) RETURNING 1
+ ) select * FROM foo f"""
+ )
+ assert sql.is_select() is False
+
+
+def test_cte_with_multiple_selects() -> None:
+ sql = ParsedQuery(
+ "WITH a AS ( select * from foo1 ), b as (select * from foo2) SELECT * FROM a;"
+ )
+ assert sql.is_select()
+
+
+def test_cte_with_multiple_with_non_select() -> None:
+ sql = ParsedQuery(
+ """WITH a AS (
+ select * from foo1
+ ), b as (
+ update foo2 set id=2
+ ) SELECT * FROM a"""
+ )
+ assert sql.is_select() is False
+ sql = ParsedQuery(
+ """WITH a AS (
+ update foo2 set name=2
+ ),
+ b as (
+ select * from foo1
+ ) SELECT * FROM a"""
+ )
+ assert sql.is_select() is False
+ sql = ParsedQuery(
+ """WITH a AS (
+ update foo2 set name=2
+ ),
+ b as (
+ update foo1 set name=2
+ ) SELECT * FROM a"""
+ )
+ assert sql.is_select() is False
+ sql = ParsedQuery(
+ """WITH a AS (
+ INSERT INTO foo (id) VALUES (1)
+ ),
+ b as (
+ select 1
+ ) SELECT * FROM a"""
+ )
+ assert sql.is_select() is False
+
+
def test_unknown_select() -> None:
"""
Test that `is_select` works when sqlparse fails to identify the type.