ctubbsii commented on code in PR #384: URL: https://github.com/apache/accumulo-website/pull/384#discussion_r1175943829
########## README.md: ########## @@ -108,6 +108,73 @@ HTML styled "just right". Jekyll will print a local URL where the site can be viewed (usually, [http://0.0.0.0:4000/](http://0.0.0.0:4000/)). +### Testing using Docker environment + +#### Build environment +A containerized development environment can be built using the local +Dockerfile. + +Run the build-images.sh script to generate the development environment and +associated images. + +```bash +./_scripts/build-images.sh +``` + +This action will produce two containers: `webdev` and `webdev-validator`. +The webdev container will execute a `jekyll serve` command with the +polling option enabled. + +This provides the ability to immediately review rendered content changes. Review Comment: I think this would expose the rendered site at `http://127.0.0.1:4000/` ? ```suggestion This provides the ability to immediately review rendered content changes at http://127.0.0.1:4000/ . ``` ########## Dockerfile: ########## @@ -0,0 +1,40 @@ +# This Dockerfile builds an ruby environment for jekyll that empowers +# making updates to the accumulo website without requiring the dev +# to maintain a local ruby development environment. + +FROM ruby:2.7.8-slim-bullseye as base + +RUN apt-get update && apt-get install -y --no-install-recommends \ + build-essential \ + git \ + curl \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /site + + +# Copy over the Gemfiles so that all build dependencies are installed +# during build vs at runtime. + +COPY Gemfile /site/Gemfile +COPY Gemfile.lock /site/Gemfile.lock + +RUN gem update --system && bundle install && gem cleanup + +ENV HOST=0.0.0.0 +ENV PORT=4000 + +EXPOSE $PORT + +CMD bundle exec jekyll serve --force-polling -H $HOST -P $PORT + +# Create a separate validation container for testing tools that +# can be used to validate the rendered HTML code. + +FROM base Review Comment: Should `base` be given a more specific name, in case somebody's Docker environment already has a container with that name, or does Docker already provide enough disambiguation for the name of running containers? ########## Dockerfile: ########## @@ -0,0 +1,40 @@ +# This Dockerfile builds an ruby environment for jekyll that empowers +# making updates to the accumulo website without requiring the dev +# to maintain a local ruby development environment. + +FROM ruby:2.7.8-slim-bullseye as base + +RUN apt-get update && apt-get install -y --no-install-recommends \ + build-essential \ + git \ + curl \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /site + + +# Copy over the Gemfiles so that all build dependencies are installed +# during build vs at runtime. + +COPY Gemfile /site/Gemfile +COPY Gemfile.lock /site/Gemfile.lock Review Comment: I'm a little confused by the name of this directory and the comment here. First, the name of the directory is "site", which is a bit confusing, because later, the site is built in `_site`. Would that location be `/site/_site/` then? Second, when is the rest of the website files copied over? Are those also copied over to the same `/site` directory inside the container? How does that work? Are they copied over to `/site` after the RUN commands but before the CMD commands? And third, just to clarify, the RUN command is using the Gemfiles to install the gems when the container image is first created and started (using `docker run`) and then the CMD commands are run. For subsequent stopping (`docker stop`) and restarting (`docker start`) , only the CMD commands are executed on startup? Is this correct? ########## README.md: ########## @@ -108,6 +108,73 @@ HTML styled "just right". Jekyll will print a local URL where the site can be viewed (usually, [http://0.0.0.0:4000/](http://0.0.0.0:4000/)). +### Testing using Docker environment + +#### Build environment +A containerized development environment can be built using the local +Dockerfile. + +Run the build-images.sh script to generate the development environment and +associated images. + +```bash +./_scripts/build-images.sh +``` + +This action will produce two containers: `webdev` and `webdev-validator`. +The webdev container will execute a `jekyll serve` command with the +polling option enabled. Review Comment: Do we always need to create both? I know I already provided feedback on the script, but given how small the `build-images.sh` script is, it might make sense to just inline the two build commands into the README, so that the second one is optional, and get rid of the script. It's basically a two line script, and the second one is optional if you're not running the validator. ########## README.md: ########## @@ -108,6 +108,73 @@ HTML styled "just right". Jekyll will print a local URL where the site can be viewed (usually, [http://0.0.0.0:4000/](http://0.0.0.0:4000/)). +### Testing using Docker environment + +#### Build environment +A containerized development environment can be built using the local +Dockerfile. + +Run the build-images.sh script to generate the development environment and +associated images. + +```bash +./_scripts/build-images.sh +``` + +This action will produce two containers: `webdev` and `webdev-validator`. +The webdev container will execute a `jekyll serve` command with the +polling option enabled. + +This provides the ability to immediately review rendered content changes. + +```bash +docker run -d -v "$PWD":/site -p 4000:4000 webdev +``` + +Shell access can be obtained by overriding the default container command. + +This is useful for adding new gems, or modifying the Gemfile.lock for updating +existing dependencies. + +```bash +docker run -v "$PWD":/site -it webdev /bin/bash +``` + +Mounting the local directory as a volume is recommended to ensure that Gemfile and +Gemfile.lock stay updated with any dependency changes. Review Comment: Is there a simple command to do that mounting that we can add to this README, or is that what is being done by the `-v` command flag? In trying to figure this out, I saw https://docs.docker.com/storage/bind-mounts/, which recommends using `--mount` instead. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
