Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: [proposal] de-matrix python-version in GHAs #27906

Merged
merged 9 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions .asf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,16 @@ github:
- cypress-matrix (2, chrome)
- cypress-matrix (3, chrome)
- frontend-build
- pre-commit (3.10)
- python-lint (3.10)
- test-mysql (3.10)
- test-postgres (3.10)
- test-postgres (3.11)
- test-postgres-hive (3.10)
- test-postgres-presto (3.10)
- test-sqlite (3.10)
- unit-tests (3.10)
- pre-commit
- python-lint
- test-mysql
- test-postgres (current)
- test-postgres (next)
- test-postgres-hive
- test-postgres-presto
- test-sqlite
- unit-tests (current)
- unit-tests (next)

required_pull_request_reviews:
dismiss_stale_reviews: false
Expand Down
37 changes: 27 additions & 10 deletions .github/actions/setup-backend/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ name: 'Setup Python Environment'
description: 'Set up Python and install dependencies with optional configurations.'
inputs:
python-version:
description: 'Python version to set up.'
description: 'Python version to set up. Accepts a version number, "current", or "next".'
required: true
default: '3.10'
default: 'current'
cache:
description: 'Cache dependencies. Options: pip'
required: false
Expand All @@ -13,22 +13,39 @@ inputs:
description: 'Type of requirements to install. Options: base, development, default'
required: false
default: 'dev'
install-superset:
description: 'Whether to install Superset itself. If false, only python is installed'
required: false
default: 'true'

runs:
using: 'composite'
steps:
- name: Set up Python ${{ inputs.python-version }}
- name: Interpret Python Version
id: set-python-version
shell: bash
run: |
if [ "${{ inputs.python-version }}" = "current" ]; then
echo "PYTHON_VERSION=3.10" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem for this PR, but I'm thinking at some point we might want to keep our npm/node/python/whatever version numbers in a JSON file at the repo's root, so they can be pulled into scripts, documentation, etc. as needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG YES! But GHA doesn't play nicely with that kind of parameter injection. Composition through the "reusable action" approach that we have here is the best we can do with basic actions (calling a reusable action that hard-codes the values).

If we wanted more flexibility I'd suggest a Jinja2-based approach, where we'd have the source templates in say a .github/jinja folder, and render that into .github/ via a commit hook or similar. Seemed overkill for now, but I'd be supportive to move in that direction.

Now comparing to where we were a few months back, we're at a much better place with this setup-backend and setup-supersetbot approach.

elif [ "${{ inputs.python-version }}" = "next" ]; then
echo "PYTHON_VERSION=3.11" >> $GITHUB_ENV
else
echo "PYTHON_VERSION=${{ inputs.python-version }}" >> $GITHUB_ENV
fi
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v5
with:
python-version: ${{ inputs.python-version }}
python-version: ${{ env.PYTHON_VERSION }}
cache: ${{ inputs.cache }}
- name: Install dependencies
run: |
sudo apt-get update && sudo apt-get -y install libldap2-dev libsasl2-dev
pip install --upgrade pip setuptools wheel
if [ "${{ inputs.requirements-type }}" = "dev" ]; then
pip install -r requirements/development.txt
elif [ "${{ inputs.requirements-type }}" = "base" ]; then
pip install -r requirements/base.txt
if [ "${{ inputs.install-superset }}" = "true" ]; then
sudo apt-get update && sudo apt-get -y install libldap2-dev libsasl2-dev
pip install --upgrade pip setuptools wheel
if [ "${{ inputs.requirements-type }}" = "dev" ]; then
pip install -r requirements/development.txt
elif [ "${{ inputs.requirements-type }}" = "base" ]; then
pip install -r requirements/base.txt
fi
fi
shell: bash
2 changes: 2 additions & 0 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ jobs:
- name: Build Docker Image
if: steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker
shell: bash
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
# Single platform builds in pull_request context to speed things up
if [ "${{ github.event_name }}" = "push" ]; then
Expand Down
96 changes: 96 additions & 0 deletions .github/workflows/no-op.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# no-op.yml
mistercrunch marked this conversation as resolved.
Show resolved Hide resolved
#
# Purpose:
# This workflow provides a workaround for the "required status checks" feature in GitHub Actions
# when using path-specific conditions in other workflows. Required checks might remain in a "Pending"
# state if the conditions are not met, thus blocking pull requests from being merged.
# This no-op (no operation) workflow provides dummy success statuses for these required jobs when
# the real jobs do not run due to path-specific conditions.
#
# How it works:
# - It defines jobs with the same names as the required jobs in the main workflows.
# - These jobs simply execute a command (`exit 0`) to succeed immediately.
# - When a pull request is created or updated, both this no-op workflow and the main workflows are triggered.
# - If the main workflows' jobs don't run (due to path conditions), these no-op jobs provide successful statuses.
# - If the main workflows' jobs do run and fail, their failure statuses take precedence,
# ensuring that pull requests are not merged with failing checks.
#
# Usage:
# - Ensure that the job names in this workflow match exactly the names of the corresponding jobs in the main workflows.
# - This workflow should be kept as-is, without path-specific conditions.

name: no-op Checks
on: pull_request

jobs:
frontend-build:
runs-on: ubuntu-latest
steps:
- name: No-op for frontend-build
run: exit 0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some inconsistent breaks between blocks if we want to get all OCD here.

I wonder: would it be a problem to add ALL required checks here? If so, I wonder if we could ingest and iterate through the list from asf.yaml rather than having to maintain this.

pre-commit:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for pre-commit
run: exit 0

python-lint:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for python-lint
run: exit 0
test-postgres-hive:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for frontend-build
run: exit 0
test-postgres-presto:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for frontend-build
run: exit 0
unit-tests:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for frontend-build
run: exit 0
test-mysql:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for test-mysql
run: exit 0
test-postgres:
strategy:
matrix:
python-version: ["3.10", "3.11"]
runs-on: ubuntu-latest
steps:
- name: No-op for test-postgres
run: exit 0
test-sqlite:
strategy:
matrix:
python-version: ["3.10"]
runs-on: ubuntu-latest
steps:
- name: No-op for test-sqlite
run: exit 0
3 changes: 0 additions & 3 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ concurrency:
jobs:
pre-commit:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v4
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/superset-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ concurrency:
jobs:
test-load-examples:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/superset-helm-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ jobs:
with:
version: v3.5.4

- uses: actions/setup-python@v5
- name: Setup Python
uses: ./.github/actions/setup-backend/
with:
python-version: "3.10"
install-superset: 'false'

- name: Set up chart-testing
uses: ./.github/actions/chart-testing-action
Expand Down
15 changes: 2 additions & 13 deletions .github/workflows/superset-python-integrationtest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ concurrency:
jobs:
test-mysql:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -51,14 +48,11 @@ jobs:
- name: Setup Python
uses: ./.github/actions/setup-backend/
if: steps.check.outputs.python
with:
python-version: ${{ matrix.python-version }}
- name: Setup MySQL
if: steps.check.outputs.python
uses: ./.github/actions/cached-dependencies
with:
run: |
setup-mysql
run: setup-mysql
- name: Run celery
if: steps.check.outputs.python
run: celery --app=superset.tasks.celery_app:app worker -Ofair -c 2 &
Expand All @@ -74,7 +68,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10", "3.11"]
python-version: ["current", "next"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -130,9 +124,6 @@ jobs:

test-sqlite:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -160,8 +151,6 @@ jobs:
- name: Setup Python
uses: ./.github/actions/setup-backend/
if: steps.check.outputs.python
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
if: steps.check.outputs.python
uses: ./.github/actions/cached-dependencies
Expand Down
10 changes: 0 additions & 10 deletions .github/workflows/superset-python-misc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ concurrency:
jobs:
python-lint:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v4
Expand All @@ -34,18 +31,13 @@ jobs:
- name: Setup Python
uses: ./.github/actions/setup-backend/
if: steps.check.outputs.python
with:
python-version: ${{ matrix.python-version }}
- name: pylint
if: steps.check.outputs.python
# `-j 0` run Pylint in parallel
run: pylint -j 0 superset

babel-extract:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v4
Expand All @@ -60,8 +52,6 @@ jobs:
- name: Setup Python
if: steps.check.outputs.python
uses: ./.github/actions/setup-backend/
with:
python-version: ${{ matrix.python-version }}
- name: Test babel extraction
if: steps.check.outputs.python
run: flask fab babel-extract --target superset/translations --output superset/translations/messages.pot --config superset/translations/babel.cfg -k _,__,t,tn,tct
6 changes: 0 additions & 6 deletions .github/workflows/superset-python-presto-hive.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ concurrency:
jobs:
test-postgres-presto:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down Expand Up @@ -84,9 +81,6 @@ jobs:

test-postgres-hive:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
env:
PYTHONPATH: ${{ github.workspace }}
SUPERSET_CONFIG: tests.integration_tests.superset_test_config
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/superset-python-unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10", "3.11"]
python-version: ["current", "next"]
env:
PYTHONPATH: ${{ github.workspace }}
steps:
Expand Down
5 changes: 0 additions & 5 deletions .github/workflows/superset-translations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ jobs:

babel-extract:
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ["3.10"]
steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
uses: actions/checkout@v4
Expand All @@ -65,8 +62,6 @@ jobs:
- name: Setup Python
if: steps.check.outputs.python
uses: ./.github/actions/setup-backend/
with:
python-version: ${{ matrix.python-version }}
- name: Test babel extraction
if: steps.check.outputs.python
run: ./scripts/babel_update.sh
Loading