upgraded gunicorn, removed deprecated flag, and added timezone to logging

[silarsis]

Upgrade of gunicorn brings it to a version that includes timezone in access log. As far as I can tell it's working - there appear to be two bugs in running the nose tests that also exist in 0.7.3, unless I'm running it wrong.

[dmp42]

Why remove the --debug switch? (here and elsewhere)

[wking]

On Tue, Aug 12, 2014 at 09:27:49AM -0700, Olivier Gambier wrote:

@@ -23,7 +23,7 @@
app.debug = True
app.run(host=host, port=port)
# Or you can run:

• gunicorn --access-logfile - --log-level debug --debug -b 0.0.0.0:5000 \

Why remove the --debug switch?

Because it's useless ;) [1,2].

[dmp42]

Removing useless stuff indeed seems like an acceptable reason (note that in v18, --debug or its absence had strange side-effects on logging IIRC) - but since we are moving up, I'm happy :-)

[dmp42]

@silarsis can you squash the two commits? (makes for a cleaner commit history)
If hacking is giving you a hard time, don't put too long a commit line.

[dmp42]

@wking I assume this is LGTM from you?
@shin- ping

[wking]

On Tue, Aug 12, 2014 at 12:18:06PM -0700, Olivier Gambier wrote:

@wking I assume this is LGTM from you?

I'd still want:

• A squash 1,
• An explanation for ‘-w 1’ → ‘-w 10’
• An explanation for ‘--max-requests 100’
• Uniformity between the README suggestion and the
  docker_registry/wsgi.py comment (or just dropping the wsgi.py
  comment).
• Dropping the trailing whitespace cleanups in the README (or
  factoring them out into a separate commit).

[wking]

On Tue, Aug 12, 2014 at 12:25:07PM -0700, W. Trevor King wrote:

Tue, Aug 12, 2014 at 12:18:06PM -0700, Olivier Gambier:

@wking I assume this is LGTM from you?

I'd still want:
…

A link to http://docs.gunicorn.org/en/19.0/news.html#core in the
commit message to motivate the --debug removal would be nice too.

[silarsis]

The second commit only exists because the first failed due to >50 char commit message - the update to -w and --max-requests just make the docco match the actual use a bit more closely (and sorry about the whitespace, I'll have words with my editor).

I can fix all the above, but newbie question - do I submit this as a new PR with a single commit, or can I cleanly adjust what's been pushed to be a single commit (and if so, how)?

Thanks :)

[wking]

On Tue, Aug 12, 2014 at 12:31:46PM -0700, Kevin Littlejohn wrote:

the update to -w and --max-requests just make the dock match the
actual use a bit more closely

Sounds good to me. Just say that in your commit message so folks
don't have to ask ;).

I can fix all the above, but newbie question - do I submit this as a
new PR with a single commit, or can I cleanly adjust what's been
pushed to be a single commit (and if so, how)?

When there's already more than one commit, I usually do something
like:

$ git rebase -i origin/master

and convert all the commits but the first from ‘pick’ to ‘squash’.
Then edit the commit message appropriately.

If you have just made one commit and realize you goofed, you can:

$ git commit --amend …

to adjust the most recent commit. Once you've fixed your local
branch, just force-push to the source of the pull-request:

$ git push -f silarsis master

assuming ‘silarsis’ is the name for your public GitHub repository.

[silarsis]

Ok, believe that's done now - let me know if there's any other alterations needed.

[wking]

On Tue, Aug 12, 2014 at 12:36:43PM -0700, W. Trevor King wrote:

$ git rebase -i origin/master

It looks like this went well :). bbca252 still has some issues,
though:

• Multi-line commit messages should have a single-line summary,
  followed by a blank line, followed by the commit message body.
  Otherwise you get things like:

  $ git log -1 --oneline bbca252

  • Upgraded gunicorn to 19.1 to gain timezones in logs * Altered docker-registry logging to include timezone * Altered documentation to more closely reflect implementation (-w and --max-requests flags as per use in run.py) * Removed unecessary documentation from wsgi.py

  If you feel the need for a bullet-point list, you may want to make a
  separate commit for each bullet point. In this case, I'd probably
  just have a single commit with a message like:

  Upgrade gunicorn to 19.1

  • Gunicorn added timezone logs in vX.X.X 1

  • Altered docker-registry logging to include timezone

  • Altered documentation to more closely reflect implementation (-w
    and --max-requests flags as per use in run.py)

  • Dropped --debug flag which is deprecated in Gunicorn 0.19 [2].

  • Removed unecessary documentation from wsgi.py (we already
    suggest how to run Gunicorn in the README).

  • Mention GUNICORN_OPTS in run.py's docstring, which should have
    happened in 2008330 (docker_registry.run: Restore GUNICORN_OPTS
    functionality, 2014-07-18).

    1: http://docs.gunicorn.org/en/…/news.html
    [2]: http://docs.gunicorn.org/en/19.0/news.html#core

• You bump ‘-w 1’ → ‘-w 10’, but the default in run.py is 4 1. I'd
  stick with 4.

[wking]

On Tue, Aug 12, 2014 at 01:12:37PM -0700, Kevin Littlejohn wrote:

Ok, believe that's done now - let me know if there's any other
alterations needed.

Ah, I see you made some of those changes in 498491e ;).

[silarsis]

I actually set the workers to 10 based on observation of running 0.7.3 out of the box - but it appears that's been changed between 0.7.3 and now, so my bad :) I've adjusted in my commit.

[wking]

On Tue, Aug 12, 2014 at 01:39:00PM -0700, Kevin Littlejohn wrote:

I actually set the workers to 10 based on observation of running
0.7.3 out of the box - but it appears that's been changed between
0.7.3 and now, so my bad :) I've adjusted in my commit.

Indeed, there seems to be some churn here. The current code base has
suggestions for:

• 9 1, set in 4963f28 (Fix Duplicate settings for which port to bind to for HTTP #368, 2014-05-30)
• 4 2, which dates back to a shift from 2 in 5a2664d (Generate
  secret_key randomly in the Dockerfile, 2013-08-23). The commit
  message for 5a2664d does not motivate the change.

I'd stick with 4.

[wking]

On Tue, Aug 12, 2014 at 01:54:35PM -0700, W. Trevor King wrote:

• 9 [1], set in 4963f28 (Fix Duplicate settings for which port to bind to for HTTP #368, 2014-05-30)

This dates back to the initial Upstart script in 50017d4 (Upstart
file for Ubuntu and others, 2013-07-02), which does not explain the
choice of 9 over 4.

[wking]

Ok, f0703e6 looks good to me :).

[dmp42]

@shin- I need a validation about the log format change - otherwise LGTM

Thanks a lot @wking and @silarsis - I like this!

[shin-]

I think that's fine. LGTM.

[dmp42]

Thanks a lot.

Merged - will do for 0.9.

Guys, keep up the excellent work!