Conversation
|
Hi, could you please remove all files from this PR except for Dockerfile, README.md, src/util.rs as those are not relevant for adding docker. I want to avoid merge conflicts. Best is probably by merging my PR and then rebasing. Dockerfile looks great. Might expose ports for Websockets, so civkit-cli and civkit-sample can connect. |
|
might replace "cargo build" with "cargo build --release --bin=civkitd" |
3ab543b to
1dbb5d3
Compare
|
Made those changes. Rebased, and exposed ports. ports can also be exposed with -p but good to include in dockerfile. Feel free to review. |
ffaex
left a comment
There was a problem hiding this comment.
looks good, should build if you do those 2 little changes
Dockerfile
Outdated
| # Copy the binaries from the builder stage | ||
| COPY --from=builder /usr/src/civkit-node/target/debug/civkitd /usr/local/bin/civkitd | ||
| COPY --from=builder /usr/src/civkit-node/target/debug/civkit-cli /usr/local/bin/civkit-cli | ||
| COPY --from=builder /usr/src/civkit-node/target/debug/civkit-sample /usr/local/bin/civkit-sample |
There was a problem hiding this comment.
path should be to release binary I think.
There was a problem hiding this comment.
i agree. if future users want to use debug in docker, they can change those lines pretty easily
| COPY --from=builder /usr/src/civkit-node/target/debug/civkitd /usr/local/bin/civkitd | ||
| COPY --from=builder /usr/src/civkit-node/target/debug/civkit-cli /usr/local/bin/civkit-cli | ||
| COPY --from=builder /usr/src/civkit-node/target/debug/civkit-sample /usr/local/bin/civkit-sample | ||
|
|
There was a problem hiding this comment.
we are compiling only the "civkitd" binary as well, so we should only copy that
There was a problem hiding this comment.
i left them thinking they could be useful for debugging / development purposes. i think we should do what you propose, keep it lean.
created barebones dockerfile to get node running. changed logging to fail gracefully is terminal is not available (like in a docker container) but logs still saved to debug file and display in docker.
it can easily be run and built.
docker build -t civkit-node .docker run --rm civkit-node