Merge branch 'main' into chapter-11-pre-review

Author: Xronophobe

.git-blame-ignore-revs

@@ -1,5 +1,5 @@
 [[chapter_10_production_readiness]]
-== Making our App Production-Ready
+== Making Our App Production-Ready
 
 .A Note for Early Release Readers
 ****
@@ -47,10 +47,13 @@ Instead, we'll use the popular Gunicorn Python/WSGI server.
 In addition, several options in _settings.py_ are currently unacceptable.
 `DEBUG=True`, is strongly recommended against for production,
 we'll want to set a unique `SECRET_KEY`,
-and as we'll see, other things will come up.
+and, as we'll see, other things will come up.
 
 Let's go through and see if we can fix things one by one.
 
+// SEBASTIAN: How about linking to Django's documentation
+//      https://docs.djangoproject.com/en/5.0/howto/deployment/
+//      somewhere later in the chapter for curious readers?
 
 === Switching to Gunicorn
 
@@ -88,6 +91,8 @@ CMD gunicorn --bind :8888 superlists.wsgi:application  <2>
   which is usually a function called `application`.
   Django provides one in _superlists/wsgi.py_.
 
+// TODO; this is a good place to switch to using array syntax for CMD maybe?
+
 As in the previous chapter, we can use the `docker build && docker run`
 pattern to try out our changes by rebuilding and rerunning our container:
 
@@ -99,6 +104,12 @@ $ *docker build -t superlists . && docker run \
   -it superlists*
 ----
 
+// DAVID: Incidentally I got the following error:
+// Bind for 0.0.0.0:8888 failed: port is already allocated.
+// Turned out the previous container was still running,
+// I just used the docker kill process you taught me about earlier.
+// Not sure if it's worth including that here, possibly clutter?
+
 
 ==== The FTs catch a problem with static files
 
@@ -187,6 +198,14 @@ $ *docker build -t superlists . && docker run \
 And if you take another manual look at your site, things should look much healthier.
 Let's rerun our FTs to confirm:
 
+// DAVID: Incidentally as per your suggestion Harry I have just been skipping
+// the requirements.txt and instead just amending the pip install in the Dockerfile.
+// If we do that, however, we'll need to make sure readers rebuild the image each time
+// we add a requirement - such as at this point.
+
+
+// JAN: That doesn't work until you install whitenoise to venv on local machine
+
 [role="small-code"]
 [subs="specialcharacters,macros"]
 ----
@@ -201,7 +220,13 @@ Ran 3 tests in 10.718s
 OK
 ----
 
-Phew.  Let's commit that
+// DAVID: I got
+// UserWarning: No directory at: /Users/david.seddon/Documents/reviewing/goat-book/src/static/
+//  mw_instance = middleware(adapted_handler)
+// I think we need to move the static folder into src too, in the previous chapter.
+
+
+Phew.  Let's commit that:
 
 [subs="specialcharacters,quotes"]
 ----
@@ -229,6 +254,8 @@ I'm sure you'll come across many others.]
 
 The `pip freeze` command will show us everything that's installed in our virtualenv at the moment:
 
+// JAN: This is true only if one installs the libraries locally. So far when following the book we installed them only inside Docker image
+
 [subs="specialcharacters,quotes"]
 ----
 $ *pip freeze*
@@ -285,11 +312,18 @@ whitenoise==6.6.0
 ====
 
 
+That's a good first cut, let's commit it:
+
+// SEBASTIAN I am not very fond of freezing just "top-level" dependencies of the project,
+// . This is not guaranteeing the reproducible builds and I failed to build & deploy because of that more than once.
+// But I understand it's a trade-off so the reader does get bogged down.
+// TODO: add a note about this and a link to some more reading
+
 ----
-# that's a good first cut, let's commit it:
 $ *git add requirements.txt*
 $ *git commit -m "Add a requirements.txt with Django, gunicorn and whitenoise"*
 ----
+
 You may be wondering why we didn't add our other dependency,
 Selenium, to our requirements,
 or why we didn't just add _all_ the dependencies,
@@ -349,6 +383,15 @@ TIP: Forgetting the `-r` and running `pip install requirements.txt`
     (which is thankfully much more helpful than it used to be).
     It's a mistake I still make, _all the time_.
 
+
+// CSANAD:  It's fine for now, but I would definitely put the requirements under
+//          /tmp and then `rm` it after `pip install`. Also, using a non-privileged
+// user is important, something like:
+//     `adduser --no-create-home --disabled-password todoapp`
+// and then setting the user in the Dockerfile with `USER todoapp`.
+// But we can cover this in a later chapter (the next one looks like a good fit,
+// since it's related to the app being production ready).
+
 Let's do a build & run & test to check everything still works:
 
 [subs="specialcharacters,quotes"]
@@ -387,12 +430,13 @@ See http://www.clearlytech.com/2014/01/04/12-factor-apps-plain-english/[
 Another common way of handling this
 is to have different versions of _settings.py_ for dev and prod.
 That can work fine too, but it can get confusing to manage.
-Environment variables also have the advantage of working for non-Django stuff too...
+Environment variables also have the advantage of working for non-Django stuff too.
 ]
 
 
 ==== Setting DEBUG=True and SECRET_KEY
 
+//RITA: What do you mean by "this"? Please add a word or two for context.
 There's lots of ways you might do this.
 Here's what I propose; it may seem a little fiddly,
 but I'll provide a little justification for each choice.
@@ -437,7 +481,7 @@ TIP: Better to fail hard than allow a typo in an environment variable name to
 
 ==== Setting environment variables inside the Dockerfile
 
-Now let's set that environment variable in our Dockerfile using then `ENV` directive:
+Now let's set that environment variable in our Dockerfile using the `ENV` directive:
 
 [role="sourcecode"]
 .Dockerfile (ch10l007)
@@ -470,7 +514,7 @@ $ pass:specialcharacters,quotes[*docker build -t superlists . && docker run \
 KeyError: 'DJANGO_SECRET_KEY'
 ----
 
-Ooops, and I forgot to set said secret key env var,
+Oops. I forgot to set said secret key env var,
 mere seconds after having dreamt it up!
 
 
@@ -505,7 +549,7 @@ AssertionError: 'To-Do' not found in 'Bad Request (400)'
 
 ==== ALLOWED_HOSTS is Required When Debug Mode is Turned Off
 
-Not quite!  Let's take a look manually: <<django-400-error>>.
+It's not quite working yet!  Let's take a look manually: <<django-400-error>>.
 
 [[django-400-error]]
 .An ugly 400 error
@@ -534,7 +578,7 @@ image::images/search-results-400-bad-request.png["Duckduckgo search results with
 `ALLOWED_HOSTS` is a security setting
 designed to reject requests that are likely to be forged, broken or malicious
 because they don't appear to be asking for your site
-(HTTP request contain the address they were intended for in a header called "Host").
+(HTTP requests contain the address they were intended for in a header called "Host").
 
 By default, when DEBUG=True, `ALLOWED_HOSTS` effectively allows _localhost_,
 our own machine, so that's why it was working OK until now.
@@ -563,7 +607,7 @@ else:
 ====
 
 This is a setting that we want to change,
-depending on whether our docker image is running locally,
+depending on whether our Docker image is running locally,
 or on a server, so we'll use the `-e` flag again:
 
 
@@ -581,7 +625,7 @@ $ *docker build -t superlists . && docker run \
 ==== Collectstatic is Required when Debug is Turned Off
 
 An FT run (or just looking at the site) reveals that we've had a regression
-in our static files.
+in our static files:
 
 [role="small-code"]
 [subs="specialcharacters,macros"]
@@ -615,6 +659,13 @@ CMD gunicorn --bind :8888 superlists.wsgi:application
 ----
 ====
 
+// DAVID: It would be nice to explain the difference between RUN and CMD.
+
+// DAVID: Interestingly when I did this I put the RUN directive after the ENV
+// directive, which led to a KeyError: 'DJANGO_SECRET_KEY' which foxed me for a bit.
+// Might be worth calling out that we're running collectstatic in debug mode.
+
+
 // TODO: gitignore src/static
 
 Well, it was fiddly, but that should get us to passing tests!
@@ -631,6 +682,9 @@ OK
 
 We're nearly ready to ship to production!
 
+// DAVID: It might be worth pointing out what Whitenoise is actually doing.
+// From what I understand, we're still using Django to serve static files.
+
 Let's quickly adjust our gitignore, since the static folder is in a new place:
 
 //0010
@@ -645,6 +699,21 @@ $ *git commit -am "Add collectstatic to dockerfile, and new location to gitignor
 ----
 
 
+=== Switching to a nonroot user
+
+TODO: WIP, this is definitely a good idea for security, needs writing up.
+
+Dockerfile should gain some lines a bit like this:
+
+.Dockerfile (ch10l0XX)
+====
+[source,dockerfile]
+----
+RUN addgroup --system nonroot && adduser --system --group nonroot
+
+USER nonroot
+----
+
 
 
 === Configuring logging
@@ -666,7 +735,6 @@ $ *rm src/db.sqlite3*
 $ *touch src/db.sqlite3*  # otherwise the --mount type=bind will complain
 ----
 
-
 Now if you run the tests, you'll see they fail;
 
 [role="small-code"]
@@ -680,6 +748,10 @@ selenium.common.exceptions.NoSuchElementException: Message: Unable to locate
 element: [id="id_list_table"]; [...]
 ----
 
+// DAVID: Got me thinking, I'm not always clear when I need to rebuild the image.
+// I would have thought I might need to do it here, but I didn't. Might be worth
+// explaining in the previous chapter when we do.
+
 And you might spot in the browser that we just see a minimal error page,
 with no debug info (try it manually if you like):
 
@@ -715,7 +787,7 @@ I mean, yes, separating the concepts of handlers and loggers and filters,
 and making it all configurable in a nested hierarchy is all well and good
 and covers every possible use case,
 but sometimes you just wanna say "just print stuff to stdout pls",
-and you wish that configuring the simplest thing was a little easier].
+and you wish that configuring the simplest thing was a little easier.].
 
 Here's pretty much the simplest possible logging config
 which just prints everything to the console (ie standard out).
@@ -743,7 +815,6 @@ Rebuild and restart our container,
 try the FT again (or submitting a new list item manually)
 and we now should see a clear error message:
 
-
 ----
 Internal Server Error: /lists/new
 Traceback (most recent call last):
@@ -759,7 +830,7 @@ Traceback (most recent call last):
 django.db.utils.OperationalError: no such table: lists_list
 ----
 
-Re-create the datase with `./src/manage.py migrate` and we'll be back to a working state.
+Re-create the database with `./src/manage.py migrate` and we'll be back to a working state.
 
 Don't forget to commit our changes to _settings.py_,
 and I think we can call it job done!
@@ -768,6 +839,8 @@ when considering production-readiness,
 we've worked in small steps and used our tests all the way along,
 and we're now ready to deploy our container to a real server!
 
+Find out how, in our next exciting installment...
+
 
 [role="pagebreak-before less_space"]
 .Production-Readiness Config

chapter_09_docker.asciidoc

@@ -702,7 +702,7 @@ It's a common tool in Ansible scripts, and the syntax is very similar to Django'
 Here's what our template for the env file will looks like:
 
 [role="sourcecode"]
-.infra/env.j2 (ch11l003)
+.infra/env.j2 (ch11l004)
 ====
 [source,python]
 ----
@@ -716,7 +716,7 @@ And here's how we use it in the provisioning script:
 
 
 [role="sourcecode small-code"]
-.infra/ansible-provision.yaml (ch11l004)
+.infra/ansible-provision.yaml (ch11l005)
 ====
 [source,yaml]
 ----
@@ -935,7 +935,7 @@ We want to map port 8888 inside the container as port 80 (the default web/http p
 on the server:
 
 [role="sourcecode"]
-.infra/ansible-provision.yaml (ch11l005)
+.infra/ansible-provision.yaml (ch11l006)
 ====
 [source,yaml]
 ----
@@ -1010,7 +1010,7 @@ Here's how
 // little incomplete.
 
 [role="sourcecode"]
-.infra/ansible-provision.yaml (ch11l006)
+.infra/ansible-provision.yaml (ch11l007)
 ====
 [source,python]
 ----
@@ -1084,8 +1084,7 @@ and reloads it automatically if it crashes.
 *******************************************************************************
 
 A few more places to look and things to try, now that we've introduced
-Podman and Systemd into the mix, should things not go according to plan:
-// CSANAD: we did not mention Podman or Systemd in this chapter
+Docker into the mix, should things not go according to plan:
 
 - You can check the Container logs using
   `docker logs superlists`.

chapter_10_production_readiness.asciidoc

@@ -162,6 +162,8 @@ image::images/git_windows_installer_choose_editor.png["Screenshot of Git install
 .Add Python to the system path from the installer
 image::images/python_install_add_to_path.png["Screenshot of python installer"]
 
+// TODO: update screenshot above for 3.12
+
 The test for all this is that you should be able to go to a Git-Bash command prompt
 and just run `python` or `pip` from any folder.
 

chapter_11_ansible.asciidoc

@@ -43,12 +43,11 @@ either via www.obeythetestinggoat.com, or via the O'Reilly Learning site.
 Congratulations!
 
 At the time of writing, the whole of _Part 1_ of the book (the first 7 chapters),
-and the first chapter in Part 2 (chapter 8)
-have been updated for the third edition.
-They’ve been upgraded to Python 3.11, Django 4,
-and the text has been comprehensively reviewed.
+and the bulk of Part 2 (chapter 8-14) have been updated for the third edition.
+They’ve been upgraded to Python 3.12, Django 4,
+and the text up to chapter 12 has been comprehensively reviewed.
 
-_Chapters 9 and above are still on Python 3.7 / Django 1.x._
+_Chapters 15 and above are still on Python 3.7 / Django 1.x._
 
 If you're following through the code examples in rest of the book,
 you have two choices:

pre-requisite-installations.asciidoc

@@ -5,6 +5,6 @@ CHAP=$1
 
 cd "$CHAP/superlists"
 git push --force-with-lease local "$CHAP"
-# git fpush origin $CHAP
+git push --force-with-lease origin "$CHAP"
 
 cd ../..