From 410ef9da060ecbfb05c212043a2b1d200bd9f6f4 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:27:13 -0800 Subject: [PATCH 1/6] fix: docker-compose-image-tag fails to start Recently I did some work to use `uv pip install` in place of plain `pip` in our docker images. This made builds significantly faster, but broke compatibility between some docker compose setups. The issue is around the fact that `docker-compose-image-tag` mounts the local, newer, docker bootstrap scripts into the image, and those have evolve to use `uv`, which isn't installed in the image. One option would be to not mount the docker folder and use what's in the image, but I believe some of the script may have changed names or location, and `docker-compose-image-tag` specifies their location as it passes arguments to them. In any case, and at least for now, I decided to simply conditionally use uv or plain pip in the docker scripts based on what's installed in the image. Also added a CI step to make sure `docker compose -f docker-compose-image-tag.yml up` works. --- .github/workflows/docker.yml | 18 ++++++++++++++++++ docker/docker-bootstrap.sh | 17 +++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 91256651f1a5a..3f6d9f05ea44e 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -25,6 +25,24 @@ jobs: echo "matrix_config=${MATRIX_CONFIG}" >> $GITHUB_OUTPUT echo $GITHUB_OUTPUT + docker-compose-image-tag: + needs: setup_matrix + runs-on: ubuntu-24.04 + steps: + - name: Setup Docker Environment + if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + uses: ./.github/actions/setup-docker + with: + dockerhub-user: ${{ secrets.DOCKERHUB_USER }} + dockerhub-token: ${{ secrets.DOCKERHUB_TOKEN }} + build: "false" + install-docker-compose: "true" + - name: docker-compose sanity check + if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + shell: bash + run: | + docker compose --exit-code-from superset-init -f docker-compose-image-tag.yml up + docker-build: name: docker-build needs: setup_matrix diff --git a/docker/docker-bootstrap.sh b/docker/docker-bootstrap.sh index 7fdb2d67a4295..1f7f17bb597df 100755 --- a/docker/docker-bootstrap.sh +++ b/docker/docker-bootstrap.sh @@ -18,13 +18,12 @@ set -eo pipefail -# UV may not be installed in older images -pip install uv - # Make python interactive if [ "$DEV_MODE" == "true" ]; then - echo "Reinstalling the app in editable mode" - uv pip install -e . + if command -v uv > /dev/null 2>&1; then + echo "Reinstalling the app in editable mode" + uv pip install -e . + fi fi REQUIREMENTS_LOCAL="/app/docker/requirements-local.txt" # If Cypress run – overwrite the password for admin and export env variables @@ -35,7 +34,13 @@ if [ "$CYPRESS_CONFIG" == "true" ]; then fi if [[ "$DATABASE_DIALECT" == postgres* ]] ; then echo "Installing postgres requirements" - uv pip install -e .[postgres] + if command -v uv > /dev/null 2>&1; then + # Use uv in newer images + uv pip install -e .[postgres] + else + # Use pip in older images + pip install -e .[postgres] + fi fi # # Make sure we have dev requirements installed From 106c2e1cc48e6d6094ab00546ac4450b95585f48 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:39:02 -0800 Subject: [PATCH 2/6] touching a file --- superset/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/__init__.py b/superset/__init__.py index cbab58e0d2c90..c97580b122cea 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -34,7 +34,8 @@ # All of the fields located here should be considered legacy. The correct way # to declare "global" dependencies is to define it in extensions.py, # then initialize it in app.create_app(). These fields will be removed -# in subsequent PRs as things are migrated towards the factory pattern +# in subsequent PRs as things are migrated towards the factory +# pattern app: Flask = current_app cache = cache_manager.cache conf = LocalProxy(lambda: current_app.config) From 23d11267017ba17a403809c62bb90a9d1a7461b0 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:41:56 -0800 Subject: [PATCH 3/6] check for file changes --- .github/workflows/docker.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 3f6d9f05ea44e..bfe472c9ac974 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -29,6 +29,11 @@ jobs: needs: setup_matrix runs-on: ubuntu-24.04 steps: + - name: Check for file changes + id: check + uses: ./.github/actions/change-detector/ + with: + token: ${{ secrets.GITHUB_TOKEN }} - name: Setup Docker Environment if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker uses: ./.github/actions/setup-docker From 8aaeae9893419a28836a24423a015738b9582a73 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:44:11 -0800 Subject: [PATCH 4/6] change ordering of jobs --- .github/workflows/docker.yml | 46 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index bfe472c9ac974..2fe30b80018b0 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -14,6 +14,7 @@ concurrency: cancel-in-progress: true jobs: + setup_matrix: runs-on: ubuntu-24.04 outputs: @@ -25,29 +26,6 @@ jobs: echo "matrix_config=${MATRIX_CONFIG}" >> $GITHUB_OUTPUT echo $GITHUB_OUTPUT - docker-compose-image-tag: - needs: setup_matrix - runs-on: ubuntu-24.04 - steps: - - name: Check for file changes - id: check - uses: ./.github/actions/change-detector/ - with: - token: ${{ secrets.GITHUB_TOKEN }} - - name: Setup Docker Environment - if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker - uses: ./.github/actions/setup-docker - with: - dockerhub-user: ${{ secrets.DOCKERHUB_USER }} - dockerhub-token: ${{ secrets.DOCKERHUB_TOKEN }} - build: "false" - install-docker-compose: "true" - - name: docker-compose sanity check - if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker - shell: bash - run: | - docker compose --exit-code-from superset-init -f docker-compose-image-tag.yml up - docker-build: name: docker-build needs: setup_matrix @@ -126,3 +104,25 @@ jobs: export SUPERSET_BUILD_TARGET=${{ matrix.build_preset }} docker compose build superset-init --build-arg DEV_MODE=false --build-arg INCLUDE_CHROMIUM=false docker compose up superset-init --exit-code-from superset-init + + docker-compose-image-tag: + runs-on: ubuntu-24.04 + steps: + - name: Check for file changes + id: check + uses: ./.github/actions/change-detector/ + with: + token: ${{ secrets.GITHUB_TOKEN }} + - name: Setup Docker Environment + if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + uses: ./.github/actions/setup-docker + with: + dockerhub-user: ${{ secrets.DOCKERHUB_USER }} + dockerhub-token: ${{ secrets.DOCKERHUB_TOKEN }} + build: "false" + install-docker-compose: "true" + - name: docker-compose sanity check + if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + shell: bash + run: | + docker compose --exit-code-from superset-init -f docker-compose-image-tag.yml up From 350a6f16fe089802900e0837fc4f468dc84d03af Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:44:41 -0800 Subject: [PATCH 5/6] checkout --- .github/workflows/docker.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 2fe30b80018b0..83f23b3ebedd3 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -108,6 +108,10 @@ jobs: docker-compose-image-tag: runs-on: ubuntu-24.04 steps: + - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" + uses: actions/checkout@v4 + with: + persist-credentials: false - name: Check for file changes id: check uses: ./.github/actions/change-detector/ From 3a1a9ca6bb9321b5b5b75cacd18158547576e3b3 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 23 Dec 2024 10:49:39 -0800 Subject: [PATCH 6/6] touchups --- .github/workflows/docker.yml | 6 +++--- scripts/change_detector.py | 2 +- superset/__init__.py | 3 +-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 83f23b3ebedd3..ea08c91eba115 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -118,7 +118,7 @@ jobs: with: token: ${{ secrets.GITHUB_TOKEN }} - name: Setup Docker Environment - if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + if: steps.check.outputs.docker uses: ./.github/actions/setup-docker with: dockerhub-user: ${{ secrets.DOCKERHUB_USER }} @@ -126,7 +126,7 @@ jobs: build: "false" install-docker-compose: "true" - name: docker-compose sanity check - if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker + if: steps.check.outputs.docker shell: bash run: | - docker compose --exit-code-from superset-init -f docker-compose-image-tag.yml up + docker compose -f docker-compose-image-tag.yml up --exit-code-from superset-init diff --git a/scripts/change_detector.py b/scripts/change_detector.py index 7394936d3b388..7efd0ede5ec84 100755 --- a/scripts/change_detector.py +++ b/scripts/change_detector.py @@ -40,7 +40,7 @@ ], "docker": [ r"^Dockerfile$", - r"^docker/", + r"^docker.*", ], "docs": [ r"^docs/", diff --git a/superset/__init__.py b/superset/__init__.py index c97580b122cea..cbab58e0d2c90 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -34,8 +34,7 @@ # All of the fields located here should be considered legacy. The correct way # to declare "global" dependencies is to define it in extensions.py, # then initialize it in app.create_app(). These fields will be removed -# in subsequent PRs as things are migrated towards the factory -# pattern +# in subsequent PRs as things are migrated towards the factory pattern app: Flask = current_app cache = cache_manager.cache conf = LocalProxy(lambda: current_app.config)