Merge pull request #212 from seddonym/review-docker

Review Docker chapter

Author: hjwp

chapter_09_docker.asciidoc

@@ -301,8 +301,11 @@ and where testing fits in.
 * Get a first cut of our code up and running inside Docker,
   with passing tests.
 
+// DAVID: might be worth condensing this bulleted list, it's rather skimmable
+// at the moment. For example is there any difference between the first and fourth point?
 
 //RITA: Consider cross-referencing the chapter by number here so we can hyperlink it for convenience.
+
 **Next chapter: Moving to a production-ready configuration**
 
 * Gradually, incrementally change the container configuration
@@ -360,7 +363,7 @@ class NewVisitorTest(StaticLiveServerTestCase):
             self.live_server_url = "http://" + test_server  #<2>
 ----
 ====
-
+// DAVID: could use a walrus operator here?
 
 Do you remember I said that `LiveServerTestCase` had certain limitations?
 Well, one is that it always assumes you want to use its own test server,
@@ -459,17 +462,21 @@ TIP: Don't use `export` to set the 'TEST_SERVER' environment variable;
 
 ==== Making an src Folder
 
+// DAVID: FWIW it reads weirdly to me to have 'an src' rather than 'a src'.
+// It's probably because I pronounce it as 'source' rather than 'S.R.C.'
+
 When preparing a codebase for deployment,
 it's often convenient to separate out the actual source code of our production app,
 from the rest of the files that you need in the project.
 A folder called _src_ is a common convention.
 
+// DAVID: I'd expect src here and below to be in monospace.
+
 Currently, all our code is source code really, so we move everything into _src_
-(we'll be seeing some new files appearing outside _src_ shortly.footnote:[
+(we'll be seeing some new files appearing outside _src_ shortly).footnote:[
 A common thing to find outside of the _src_ folder is a folder called _tests_.
 We won't be doing that while we're relying on the standard Django test framework,
 but it's a good thing to do if you're using pytest, for example.]
-)
 
 
 
@@ -516,6 +523,13 @@ Cool! We'll find out more about all of these steps as the chapter progresses.
 Impartiality commands me to also recommend https://podman.io/[Podman],
 which is a like-for-like replacement for Docker.
 
+// DAVID: It might be worth mentioning Colima, which I have running on my machine
+// instead of Docker because of licensing restrictions. I was able to run the
+// same command because I have Colima installed. The good thing about Colima
+// is you just use the same `docker` command, don't need to change it.
+// Also - possibly worth moving the alternatives a couple of paras further
+// up, so that they know their options before running the Docker install?
+
 It's pretty much exactly the same as Docker,
 arguably with a few advantages even, but I won't go into detail here.
 
@@ -527,7 +541,7 @@ Docker is open source too,
 so I decided to stick with it. But you could definitely check it out!
 
 You can follow along all the instructions in the book
-by just substituing the `docker` binary for `podman` in all the CLI instructions,
+by just substituting the `docker` binary for `podman` in all the CLI instructions,
 e.g.
 
 [role="skipme"]
@@ -550,6 +564,9 @@ including the operating system, dependencies, and any code you want to run.
 
 Once you have an image, you can run one or many containers that use the same image.
 
+// DAVID: Not sure whether this is going to be understandable in its current form.
+// An analogy that I find helpful here is that images are like classes
+// and containers are like instances.
 
 ==== A First Cut of a Dockerfile
 
@@ -566,6 +583,11 @@ What do we need to do?  Something like this, right?
 We create a new file called _Dockerfile_ in the base folder of our repo,
 next to the `src/` directory we made earlier:
 
+// DAVID: As discussed, I misread this and put the Dockerfile in src.
+// That led to a difficult to understand error message when I tried to build.
+// ERROR: failed to solve: failed to compute cache key: failed to calculate
+// checksum of ref e126cbb4-3e0c-4e6e-867f-dbfbf5fb190f::k2oys44pcnbefrlpps3ooux7w: "/src": not found
+
 // JAN: I'd suggest to use python3.11:slim or python3:12 slim. Keeping image tags too open leads to issues in a couple of months (not always, but waaay too often)
 // JAN: I'd use Docker comments with # for <1>, <2>, ... Otherwise, you need to edit the code when you paste it
 
@@ -711,6 +733,9 @@ environment?
 ----
 
 
+// DAVID: I got:
+// python: can't open file '/src/manage.py': [Errno 2] No such file or directory
+
 Ah, we forgot that we need to install Django.
 
 
@@ -754,6 +779,7 @@ CMD python manage.py runserver
 <3> We install Django with `pip install`, just like we do locally.
 
 
+// DAVID: Why bother with a virtualenv?
 
 ==== Successful Run
 
@@ -805,13 +831,16 @@ WARNING: Make sure you use the `-it` flags to the Docker `run`
     to interrupt the docker process with _Ctrl-C_.
     See <<how-to-stop-a-docker-container>> for an escape hatch.
 
+// DAVID: behaviours -> behaviour?
 
 [[how-to-stop-a-docker-container]]
 .How to Stop a Docker Container
 *******************************************************************************
 If you've got a container that's "hanging" in a terminal window,
 you can kill it from another one.
 
+// DAVID: are we actually killing it from another container?
+
 The docker daemon lets you list all the currently running containers
 with `docker ps`:
 
@@ -1032,7 +1061,10 @@ $ *docker build -t superlists . && docker run -p 8888:8888 -it superlists*
 Now that will _change_ the error we see, but only quite subtly (see <<firefox-connection-reset>>).
 Things clearly aren't working yet.
 
+// DAVID: FWIW in Chrome the message is "127.0.0.1 didn’t send any data."
+
 //RITA: If at all possible, I suggest using the light or daytime theme for all browser screenshots to make them easier to read.
+
 [[firefox-connection-reset]]
 .Cannot connect on that port
 image::images/firefox-connection-reset.png["Firefox showing the 'Connection reset' error"]
@@ -1107,6 +1139,9 @@ The long and short of it is that
 we need use the long-form `ipaddr:port` version of the `runserver` command,
 using the magic "wilcard" IP address `0.0.0.0`:
 
+// DAVID: Some readers might not be that comfortable with the idea of ports.
+// I think this could do with more explanation.
+
 .Dockerfile (ch09l007)
 ====
 [source,dockerfile]
@@ -1128,6 +1163,7 @@ $ *docker build -t superlists . && docker run -p 8888:8888 -it superlists*
 Starting development server at http://0.0.0.0:8888/
 ----
 
+
 We can verify it's working with `curl`:
 
 [subs="specialcharacters,macros"]
@@ -1215,6 +1251,8 @@ we highlighted as one of the "danger areas" of deployment).
 You might have spotted the yellow Django debug page (<<django-debug-screen>>)
 telling us as much, or if you tried it manually.
 
+// DAVID: Grammar on that last sentence?
+
 NOTE: The tests saved us from potential embarrassment there.
     The site _looked_ fine when we loaded its front page.
     If we'd been a little hasty and only testing manually,
@@ -1234,6 +1272,8 @@ To be fair, if you look back through the `runserver` command output
 each time we've been starting our container,
 you'll see it's been warning us about this issue:
 
+// DAVID: mix between 'you' and 'our' reads weirdly.
+
 [role="skipme"]
 ----
 You have 19 unapplied migration(s). Your project may not work properly until
@@ -1295,6 +1335,9 @@ OK
 But we don't actually want to package up our database _inside_ the image, do we?
 We want the database on the server to have totally separate data from the one on our machine.
 
+// DAVID: This is an important point which might need a bit more explanation.
+// What would happen if we did?
+
 In most deployments, you'd probably be talking to a separate database server, like postgres.
 
 For the purposes of this book, the easiest analogy to a server that's "outside" our container,
@@ -1388,7 +1431,7 @@ Ahem, that's definitely good enough for now!  Let's commit.
 $ *git add Dockerfile*
 $ *git commit -m"First cut of a Dockerfile"*
 ----
-
+// DAVID: This is the second cut really?
 
 Phew.  Well, it took a bit of hacking about,
 but now we can be reassured that the basic Docker plumbing works.
@@ -1402,11 +1445,13 @@ In the next chapter, we'll make our hacky image more production-ready.
 But first, time for a well-earned tea break I think, and perhaps a
 https://en.wikipedia.org/wiki/Digestive_biscuit[chocolate biscuit].
 
+// DAVID: That ain't no chocolate biscuit!
 
 .Test-Driving Server Configuration and Deployment
 *******************************************************************************
 
 Tests and small steps some of the uncertainty out of deployment::
+// DAVID: wording here?
     For developers, ops and infra work is always "fun",
     by which I mean a process full of fear, uncertainty and surprises.
     My aim during this chapter was to show that a step-by-step approach