ddanielr commented on code in PR #384:
URL: https://github.com/apache/accumulo-website/pull/384#discussion_r1175922848


##########
Dockerfile:
##########
@@ -0,0 +1,30 @@
+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 Gemfile /site/Gemfile
+COPY Gemfile.lock /site/Gemfile.lock
+
+RUN gem update --system && bundle install && gem cleanup

Review Comment:
   > I'm not super familiar with the base ruby image, but in my environment, 
the system gems are provided by system RPMs in my distribution. This has been 
my experience using CentOS/RHEL/Fedora. So, a `yum` (or `dnf`) update is 
normally sufficient and I don't need to do a `gem update`. Is the situation 
different for the base ruby image?
   > 
   The ruby image uses RubyGems for dependency management. In the Dockerfile we 
run the `apt-get update` command first to ensure that system packages are 
updated and also install some basic required packages. 
   
   `gem update --system` ensures that any Gems loaded from RubyGems are also 
updated before we attempt to install our application specific gems via `bundle 
install`. 
   
   Your comment got me curious so I popped into the image and verified there 
are no ruby-related packages installed via `apt`.  
   
   > Also, the instructions on the README in 
https://github.com/apache/accumulo-website#local-builds-for-testing explain in 
detail how to set up the GEM environment as a non-supervisor user for local 
gems installed to the user directory by bundler. I think it would be helpful if 
this Dockerfile did something similar, just automated, so instead of setting 
things up differently by doing system installed gems. That way, there's only a 
single set of instructions to follow, whether we're doing something via a local 
build, or trying to maintain this Dockerfile in future. However, it looks like 
this is relying on a different set of environmental assumptions from the those 
in the README. It looks like this is installing gems using the superuser, and 
gems are being installed to a system directory rather than a local user 
directory. I'm not 100% certain of this, since I'm not super familiar with 
Docker. If this is diverging from the detailed README instructions, it would be 
helpful to have som
 e inline comments explaining what's being done here, so we have a better 
chance of being able to maintain it going forward.
   
   Docker is operating on it's own separate filesystem layer so nothing 
installed in the container will affect anything on the given system. This 
accomplishes the same separation as setting `GEM_HOME` to a user directory. 
   
   If you're interested, the [Ruby image spec](https://hub.docker.com/_/ruby) 
calls out various environment variables that are set in the image, including 
`GEM_HOME` which is set to `/usr/local/bundle`. 
   
   The explicit `gem install bundler` command isn't needed as the Gemfile.lock 
requires Ruby 2.7 to be the 
   minimum installed and bundler was added as a default gem in 2.6. 
   
   So, I don't see this diverging from the detailed readme instructions as 
`GEM_HOME` is already set in the container and the next command to run is 
`bundle install`. 
   
   As long as all of the required dependencies are defined in the Gemfile, then 
we can ensure that all development environments are held to the same standard. 
   
   The **only** action that would require an update to the Dockerfile is a Ruby 
version update. But that is still extremely simple as you would just update the 
`FROM` image tag and run the documented `_/scripts/build-images.sh` script .
   



-- 
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]

Reply via email to