Review chapter 10

Author: seddonym

chapter_10_production_readiness.asciidoc

@@ -41,7 +41,7 @@ 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.
 
@@ -81,6 +81,8 @@ CMD gunicorn --bind :8888 superlists.wsgi:application
 ----
 ====
 
+// DAVID: Does this need a bit more explanation?
+
 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:
 
@@ -92,7 +94,11 @@ $ *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?
 
 
 
@@ -169,6 +175,11 @@ MIDDLEWARE = [
 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.
+
 [role="small-code"]
 [subs="specialcharacters,macros"]
 ----
@@ -182,6 +193,11 @@ Ran 3 tests in 10.718s
 OK
 ----
 
+// 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"]
@@ -268,7 +284,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 (ch10l006)
@@ -364,7 +380,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.
@@ -444,6 +460,12 @@ 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.
+
 Well, it was fiddly, but that should get us to passing tests!
 
 
@@ -457,6 +479,9 @@ OK
 
 We have a container that we're 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.
+
 Find out how in the next exciting installment...
 
 
@@ -482,7 +507,7 @@ $ *rm src/db.sqlite3 && touch src/db.sqlite3*
 TIP: If you use the `-v` option for docker run to mount a nonexistent path,
     Docker will create a new folder at that path.
     This is why we do the `touch db.sqlite3`
-    so that we have a placeholder empty file where the datbase should be.
+    so that we have a placeholder empty file where the database should be.
     Otherwise we get a folder instead of an empty file and django gets even more confused;
     if you ever see an error saying `chown: permission denied`, that's probably it.
     Do a `rm -rf src/db.sqlite3 && touch src/db.sqlite3` to fix it.
@@ -501,6 +526,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):
 
@@ -536,7 +565,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).
@@ -564,7 +593,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):