Gslux 635 use v4 release bundle #3180
Conversation
geoportal/Dockerfile
Outdated
| RUN curl -L https://github.com/Geoportail-Luxembourg/luxembourg-geoportail/releases/download/$(cat v4_release.txt)/bundle.tgz > /tmp/bundle.tgz && \ | ||
| tar -xzvf /tmp/bundle.tgz -C /tmp && \ | ||
| mkdir -p /usr/lib/node_modules/luxembourg-geoportail && \ | ||
| mv /tmp/bundle/ /usr/lib/node_modules/luxembourg-geoportail |
There was a problem hiding this comment.
I'm not a big fan of manipulating the npm dependencies "manually" here, as the package.json is already there to manage such things. Maybe we could find a way to continue making the bundle available that is directly accessible for the package.json (in a dedicated branch or via npm for example).
There was a problem hiding this comment.
I agree with you.
There are three arguments in favor of this approach, though:
- it's "less bad" than before with the bundle inside the repo and the whole source files installed and unused
- it's only for the time of the backport while v3 and v4 coexist. And it's by far not the only hack for this time
- in case of a V4 change, the docker image only has to be rebuilt for the last stages, the (long) installation of basic V3 npm dependencies can be skipped
so in the end I do not feel too bad with this "hack", but I'll check if we can build a "fake" npm package to install via the git hash...
There was a problem hiding this comment.
OK, thanks for your great idea, so we can use npm to install the bundle, that gives more transparency
|
|
||
| COPY . /app | ||
| RUN mkdir -p /app | ||
| COPY ./package.json /app | ||
|
|
||
| # jsapi generation | ||
| ADD ./jsapi /etc/apiv4/ |
There was a problem hiding this comment.
I think we can get rid of the diff of the Dockerfile completely, as the npm install in between the two changes only concerns the JS API, if I see correctly. The package.json for the app is actually already copied in line 28.
There was a problem hiding this comment.
the main point of this diff is removing the COPY . /app and putting it after the jsapi build
but yes, we can remove
RUN mkdir -p /app
COPY ./package.json /app
There was a problem hiding this comment.
Ok I don't get the advantage of moving COPY . /app, neither as the WORKDIR changes, but I might be missing something.
There was a problem hiding this comment.
By moving the cp . as late as possible we avoid all unnecessary docker build steps (npm install for jsapi and build jsapi) if the source code changes
This is just a build optimization which may reduce the image build time if something changes in app
We can leave the Dockerfile as is if that is more comfortable for you.
There was a problem hiding this comment.
Alright, I'm fine with moving the COPY . /app
needs the published bundles in Geoportail-Luxembourg/luxembourg-geoportail#128