-
Notifications
You must be signed in to change notification settings - Fork 178
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
New Dockerfile #849
base: master
Are you sure you want to change the base?
New Dockerfile #849
Changes from 8 commits
53924d5
44af1c5
8d75105
61ddc99
a41aaee
4b1b634
d3dd12c
9de2591
ec930a4
df73b96
c8f31c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
FROM node:6.17.1 | ||
|
||
COPY . /voyager | ||
|
||
RUN cd voyager && yarn && yarn build | ||
|
||
WORKDIR /voyager | ||
|
||
EXPOSE 9000 | ||
|
||
CMD ["yarn", "start:headless"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
Build Docker image | ||
``` | ||
docker build -t vega/voyager . | ||
``` | ||
|
||
Run Docker container. The `-p 9000:9000` parameter is required to publish the container's yarn port to a host port, allowing users to view the datavoyager tool at http://localhost:9000/. | ||
``` | ||
docker run -p 9000:9000 vega/voyager | ||
``` | ||
|
||
Alternatively, run the Docker container detatched with `-d` to not receive log information in the console. | ||
``` | ||
docker run -d -p 9000:900 vega/voyager | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use node 12 or 14? Node 6 is not maintained anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should switch to github actions like we did in all other active vega projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since node 12 is the oldest supported version, let's hold off on merging this pull request.
https://nodejs.org/en/about/releases/