New Dockerfile #849
New Dockerfile #849
Conversation
…during testing, add start:headless to reduce warning messages when running Docker image
Dockerfile
Outdated
| @@ -0,0 +1,11 @@ | |||
| FROM node:6.17.1 | |||
There was a problem hiding this comment.
Can you use node 12 or 14? Node 6 is not maintained anymore.
There was a problem hiding this comment.
I picked 6.17.1 as it is what the Travis CI build uses. The newer versions of Node are incompatible with vega/voyager -- at least the builds failed for me. It looks like the voyager code uses removed APIs (likely long past deprecated).
The knock/voyager Dockerfile I based this on had used node 9
There was a problem hiding this comment.
Maybe we should switch to github actions like we did in all other active vega projects.
There was a problem hiding this comment.
Certainly an option, but I think you'd still need to build with the older version of node until the voyager software is updated. The .travis.yml defines the version of node used, not Travis itself. https://github.com/vega/voyager/blob/master/.travis.yml#L3
There was a problem hiding this comment.
A quick change to the Dockerfile to use node:latest (presumably 14.4) and it also looks like there are library version incompatibilities. I had tried earlier today to force upgrading all npm libraries to current/latest, but that was unsuccessful too. I hadn't spent the time to dive deeper into the software to determine what versions of each of the dependencies are required. I had really just wanted to get it working, so I fell back to the known working & tested version of 6.17.1.
There was a problem hiding this comment.
Since node 12 is the oldest supported version, let's hold off on merging this pull request.
README.md
Outdated
| @@ -156,3 +156,21 @@ This will run Voyager in "server-mode" sending requests to voyager-server, which | |||
| The server url is controlled by the `SERVER` environment variable. | |||
|
|
|||
| See [voyager-server](https://github.com/vega/voyager-server) for more information on what portions of the functionality the server handles. | |||
|
|
|||
| ## Docker | |||
| Docker image based on [node:6.17.1](https://hub.docker.com/layers/node/library/node/6.17.1/images/sha256-0bb1f1235ffb537f2543de5fa0de7e4e93f522dd9cc6da51da255cffbdd99aa3?context=explore) | |||
There was a problem hiding this comment.
Would it make sense to use VSCode's container development feature instead? https://code.visualstudio.com/docs/remote/containers
There was a problem hiding this comment.
I'm not familiar with VSCode's development container feature. It certainly looks interesting.
In my case, my deployment environment requires the Docker container, though I could potentially see the same container reused for development testing & debugging.
There was a problem hiding this comment.
Okay. I'll let @kanitw make the call whether he wants a docker container in master right now. Even if not, it'll be great to have this pull request as a reference if someone wants to use docker.
|
I brought in the same changes from PR #839 to updated node-sass. That lets it build with Node 14.4.0. I also changed the .travis-ci.yml and Dockerfile to use 14.4.0 I didn't change any of the other dependencies -- quite a few are out of date. I took a couple of hours yesterday to try to just update all to the latest and quickly found there are code and dependency incompatibilities. I made it through a few of them, but decided to stop when I got pretty deep into the code. I figure it would be better left to a person more familiar with the code base. |
--openparameter as it isn't needed in DockerWhile running Travis CI build on my fork to verify my changes didn't break anything tested, I found the build has an out of memory error. I increased the
--max-old-space-sizeto work around the issue.