From 772d37ead363f55b4c2a5b2e15a41495b45991fc Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 21 Mar 2024 10:22:14 -0700 Subject: [PATCH 1/5] fix(docker): error around missing requirements/base.txt Closes https://github.com/apache/superset/issues/27606 I'm unclear on why exactly CI succeeded while this was an issue running locally. I'm guessing it has to do with `--mount` and how it works around caching, so I'm moving to a COPY-based approach, which is more standard and easy to reason about. It also leaves a useful artefact on the machine as to how it was built. It does add a tiny layer and payload on the image, but it's super cheap for the utility/introspection it provides. --- Dockerfile | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index e5387d677944a..5e7dceed4d836 100644 --- a/Dockerfile +++ b/Dockerfile @@ -61,7 +61,7 @@ ENV LANG=C.UTF-8 \ SUPERSET_HOME="/app/superset_home" \ SUPERSET_PORT=8088 -RUN mkdir -p ${PYTHONPATH} superset/static superset-frontend apache_superset.egg-info requirements \ +RUN mkdir -p ${PYTHONPATH} superset/static requirements superset-frontend apache_superset.egg-info requirements \ && useradd --user-group -d ${SUPERSET_HOME} -m --no-log-init --shell /bin/bash superset \ && apt-get update -qq && apt-get install -yqq --no-install-recommends \ build-essential \ @@ -79,8 +79,8 @@ RUN mkdir -p ${PYTHONPATH} superset/static superset-frontend apache_superset.egg COPY --chown=superset:superset setup.py MANIFEST.in README.md ./ # setup.py uses the version information in package.json COPY --chown=superset:superset superset-frontend/package.json superset-frontend/ -RUN --mount=type=bind,target=./requirements/base.txt,src=./requirements/base.txt \ - --mount=type=cache,target=/root/.cache/pip \ +COPY --chown=superset:superset requirements/base.txt requirements/ +RUN --mount=type=cache,target=/root/.cache/pip \ pip install --upgrade setuptools pip && \ pip install -r requirements/base.txt @@ -126,8 +126,9 @@ RUN apt-get update -qq \ && ln -s /opt/firefox/firefox /usr/local/bin/firefox \ && apt-get autoremove -yqq --purge wget && rm -rf /var/[log,tmp]/* /tmp/* /var/lib/apt/lists/* # Cache everything for dev purposes... -RUN --mount=type=bind,target=./requirements/development.txt,src=./requirements/development.txt \ - --mount=type=cache,target=/root/.cache/pip \ + +COPY --chown=superset:superset requirements/development.txt requirements/ +RUN --mount=type=cache,target=/root/.cache/pip \ pip install -r requirements/development.txt USER superset From 4fdf5a52da9743bdbf8830d67d601bfb515de90a Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 21 Mar 2024 11:36:33 -0700 Subject: [PATCH 2/5] fix 'supersetbot docker' preset argument --- .github/supersetbot/src/cli.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/supersetbot/src/cli.js b/.github/supersetbot/src/cli.js index b8344d6ac3e1a..4b4612ee97f1f 100755 --- a/.github/supersetbot/src/cli.js +++ b/.github/supersetbot/src/cli.js @@ -154,16 +154,16 @@ export default function getCLI(context) { program.command('docker') .description('Generates/run docker build commands use in CI') - .option('-t, --preset', 'Build preset', /^(lean|dev|dockerize|websocket|py310|ci)$/i, 'lean') + .option('-t, --preset ', 'Build preset', /^(lean|dev|dockerize|websocket|py310|ci)$/i, 'lean') .option('-c, --context ', 'Build context', /^(push|pull_request|release)$/i, 'local') .option('-r, --context-ref ', 'Reference to the PR, release, or branch') .option('-p, --platform ', 'Platforms (multiple values allowed)') .option('-f, --force-latest', 'Force the "latest" tag on the release') .option('-v, --verbose', 'Print more info') - .action(function (preset) { - const opts = context.processOptions(this); + .action(function () { + const opts = context.processOptions(this, ['preset']); opts.platform = opts.platform || ['linux/arm64']; - const cmd = docker.getDockerCommand({ preset, ...opts }); + const cmd = docker.getDockerCommand({ ...opts }); context.log(cmd); if (!opts.dryRun) { utils.runShellCommand(cmd, false); From 1bebd507438a14ac4abe0238736dfbc8136b963b Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 21 Mar 2024 11:45:52 -0700 Subject: [PATCH 3/5] add fix to supersetbot + unit test --- .github/supersetbot/src/cli.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .github/supersetbot/src/cli.test.js diff --git a/.github/supersetbot/src/cli.test.js b/.github/supersetbot/src/cli.test.js new file mode 100644 index 0000000000000..c2f1e8ecf554a --- /dev/null +++ b/.github/supersetbot/src/cli.test.js @@ -0,0 +1,19 @@ +import { spawnSync } from 'child_process'; + +describe('CLI Test', () => { + it('should contain "--target dev" in the output when calling "supersetbot docker --preset dev --dry-run"', () => { + // Run the CLI command + const result = spawnSync('./src/supersetbot', ['docker', '--preset', 'dev', '--dry-run']); + + if (result.error) { + // Handle the error, for example, by failing the test + throw new Error(result.error.message); + } + + // Convert the stdout buffer to a string + const output = result.stdout.toString(); + + // Check if the output contains "--target dev" + expect(output).toContain('--target dev'); + }); +}); From 89ffd273ed1c49c13ce9b981d2260b786924b70c Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 21 Mar 2024 11:52:35 -0700 Subject: [PATCH 4/5] more tests --- .github/supersetbot/src/cli.test.js | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/.github/supersetbot/src/cli.test.js b/.github/supersetbot/src/cli.test.js index c2f1e8ecf554a..60ad8c94cddc8 100644 --- a/.github/supersetbot/src/cli.test.js +++ b/.github/supersetbot/src/cli.test.js @@ -1,19 +1,12 @@ import { spawnSync } from 'child_process'; describe('CLI Test', () => { - it('should contain "--target dev" in the output when calling "supersetbot docker --preset dev --dry-run"', () => { - // Run the CLI command - const result = spawnSync('./src/supersetbot', ['docker', '--preset', 'dev', '--dry-run']); - - if (result.error) { - // Handle the error, for example, by failing the test - throw new Error(result.error.message); - } - - // Convert the stdout buffer to a string + test.each([ + ['./src/supersetbot', ['docker', '--preset', 'dev', '--dry-run'], '--target dev'], + ['./src/supersetbot', ['docker', '--dry-run'], '--target lean'], + ])('returns %s for release %s', (command, arg, contains) => { + const result = spawnSync(command, arg); const output = result.stdout.toString(); - - // Check if the output contains "--target dev" - expect(output).toContain('--target dev'); + expect(result.stdout.toString()).toContain(contains); }); }); From 900719c7ec235ee9fb3c1e12977a45ce1dc9f8e9 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 21 Mar 2024 11:55:26 -0700 Subject: [PATCH 5/5] bump supersetbot --- .github/supersetbot/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/supersetbot/package.json b/.github/supersetbot/package.json index ee9d008801f9d..af6469a26bb8e 100644 --- a/.github/supersetbot/package.json +++ b/.github/supersetbot/package.json @@ -1,6 +1,6 @@ { "name": "supersetbot", - "version": "0.4.1", + "version": "0.4.2", "description": "A bot for the Superset GitHub repo", "type": "module", "main": "src/index.js",