feat: skip npm run build in docker-compose
#28784
Conversation
mistercrunch
force-pushed
the
docker-compose-mem
branch
from
5e11a08 to
a037042
Compare
mistercrunch
force-pushed
the
docker-compose-mem
branch
from
a037042 to
4960603
Compare
When using docker-compose, we mount the local drive and fire up a `npm run dev` which is effectively webpack in --watch mode, so that it looks for changes in frontend files and compiles-as-you-change interactively. Now, somehow `npm run build` is in the Dockerfile, specifically in the superset-node layer which is used to compile and later COPY the static assets into a python-based layer, `lean`. Now in the context of docker-compose, we don't need all of it, and that `npm run build` has gotten expensive in terms of memory usage and takes a while to run. So in this PR I'm: - making `npm run build` optional with a dockerfile ARG (default is current behavior, but docker-compose skips it) - adding a `superset-node-base` layer for that superset-node service in docker-compose that runs `npm run dev`, ensuring the same base for both processes, but stopping/diverging prior to `npm ci` - making `npm install` run only if node_module is not present in the superset-node service. The user can simply `npm i` locally if/when they alter package.json. That speeds up firing up `docker-compose up` as unless you nuke node_modules/ it'll skip that step
mistercrunch
force-pushed
the
docker-compose-mem
branch
from
4960603 to
6541ff6
Compare
superset-frontend/webpack.config.js
Outdated
| @@ -496,7 +496,7 @@ const config = { | |||
| 'react/lib/ReactContext': true, | |||
| }, | |||
| plugins, | |||
| devtool: 'source-map', | |||
| devtool: isDevMode ? 'source-map' : false, | |||
There was a problem hiding this comment.
read somewhere that turning off source maps in prod may help memory usage, but also thinking it's better to not have source maps in prod as it's lighter weight / more secure
There was a problem hiding this comment.
The source maps are SUPER helpful in prod, actually... it allows us to help pinpoint runtime errors in the codebase rather than see garbage minimized JS and stacktraces. I would leave these on.
| @@ -147,7 +149,11 @@ services: | |||
| disable: true | |||
|
|
|||
| superset-node: | |||
| image: node:18 | |||
| build: | |||
There was a problem hiding this comment.
This makes things a bit more DRY and in sync with the Dockerfile, main thing is the base is set in docker for this, and there's a frontend-mem-nag script that checks some assumption and raises message if docker conditions aren't set properly.
There was a problem hiding this comment.
That memory nag seems to be set to 3GB. Seems like we might as well raise it, no? Not sure what a reasonable minimum is at this point, but something significant - maybe 10GB?
There was a problem hiding this comment.
ok let's try 10, unclear what exactly is needed and different contexts, but 10 seems much more accurate than 3
There was a problem hiding this comment.
if people have ideas as to how to contain this docker beast it'd be great
|
@dpgaspar @craig-rueda curious what you think here |
|
Source maps are useful, but are they out of place in production? Though clearly that's a total bycatch and probably doesn't belong in this PR. Guessing that for a non-open source context it's bad practice to expose your source, but for open source it probably matters less. |
|
Actually I think I may have been misled by GPT and thought docker was smarter than it is, as an immutable system it's hard to do anything conditional in docker. I think we'd have to remodel things quite a bit and have multiple docker files to stay sane while doing this. |
When using docker-compose, we mount the local drive and fire up a
npm run devwhich is effectively webpack in --watch mode, so that it looks for changes in frontend files and compiles-as-you-change interactively.npm run buildis in the Dockerfile, specifically in the superset-node layer which is used to compile and later COPY the static assets into a python-based layer,lean.But in the context of docker-compose, we don't need all of it, and that
npm run buildhas gotten expensive in terms of memory usage and takes a while to build. In recent build memory monitoring I've seen the docker container peak at like 13GB memory during this phase.So in this PR I'm:
npm run buildoptional with a dockerfile ARG (default is current behavior, but docker-compose skips it)superset-node-baselayer for that superset-node service in docker-compose that runsnpm run dev, ensuring the same base for both processes, but stopping/diverging prior tonpm cinpm installrun only if node_module is not present in the superset-node service. The user can simplynpm ilocally if/when they alter package.json. That speeds up firing updocker-compose upas unless you nuke node_modules/ it'll skip that step