Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Upgrade Aspen #2384

Merged
merged 24 commits into from
Jul 9, 2014
Merged

Upgrade Aspen #2384

merged 24 commits into from
Jul 9, 2014

Conversation

Changaco
Copy link
Contributor

@whit537 I need your help on this one.

@chadwhitacre
Copy link
Contributor

The biggest change is that we'll need to install uwsgi or gunicorn or something (#1868) and start using that, because in 0.31 we ripped out Aspen's built-in app server.

@chadwhitacre
Copy link
Contributor

I'm looking at this.

@chadwhitacre
Copy link
Contributor

Using either uswgi or gunicorn, I get an error trying to connect to postgres.

@chadwhitacre
Copy link
Contributor

It's difficult to debug because I can't get to pdb because of the process boundary.

@chadwhitacre
Copy link
Contributor

psql connects just fine.

Changaco added a commit that referenced this pull request Jun 11, 2014
@Changaco
Copy link
Contributor Author

Rebased on master and added the commit to remove our temporary copy of to_rfc822().

@chadwhitacre
Copy link
Contributor

Rebased on master.

@chadwhitacre
Copy link
Contributor

@Changaco So the reason this is going slower than expected is because we made changes affecting JSON simplates. We did away with JSON simplates as a special case; now they're regular simplates that use a json_dump renderer. I'm upgrading our JSON simplates to use this new pattern.

@chadwhitacre
Copy link
Contributor

cc: @pjz

@Changaco
Copy link
Contributor Author

@whit537 That's okay, this PR isn't blocking anything else now, so there's no rush to land it.

@chadwhitacre
Copy link
Contributor

There are some JSON files that must not be covered by the test suite, because with that last commit I fixed all the SyntaxErrors I was seeing, and there are *.json.spt files that I didn't touch there.

Changaco and others added 11 commits July 7, 2014 12:33
I went with gunicorn for a web server, because it is much simpler to
configure than uwsgi. Note that this Makefile requires a patched version
of Honcho. I made a PR but I don't know if it will be accepted:

nickstenning/honcho#71
With this newer version of Aspen we no longer have special-cased JSON
simplates. Instead they are now regular rendered/negotiated simplates
that utilize a json_dump renderer.
This version fixes a bug where we couldn't just do `honcho run -e
foo.env py.test`.
We're not using Cheroot anymore.
@Changaco Changaco changed the title Upgrade Aspen to 0.32 Upgrade Aspen Jul 8, 2014
@chadwhitacre
Copy link
Contributor

@Changaco Your additional commits look good so far. Still need to sort out js tests, looks like?

@chadwhitacre
Copy link
Contributor

IRC

@chadwhitacre
Copy link
Contributor

I'm debugging the Gruntfile gunicorn killing issue.

@chadwhitacre
Copy link
Contributor

The problem is that when the aspen:start task completes, the gunicorn parent is orphaned. It should be terminated.

@chadwhitacre
Copy link
Contributor

Hypothesis: the SIGTERM never makes it from grunt (node) to the child process (honcho).

Test: Stub out a server.py with a SIGTERM handler and see if it terminates.

@chadwhitacre
Copy link
Contributor

#!/usr/bin/env python
import signal
import sys
import time

out = open('foo.log', 'w+')

def handler(signum, frame):
    print >> out, 'Killed!'
    raise SystemExit

signal.signal(signal.SIGTERM, handler)


slept = 0
while 1:
    print >> out, "Still here."
    time.sleep(0.3)
    slept += 1
    if slept == 3:
        print >> sys.stderr, "Starting gunicorn "

@chadwhitacre
Copy link
Contributor

Result: No! The server process does receive the SIGTERM from node.

[gittip] $ ./node_modules/.bin/grunt aspen:start
Running "aspen:start" task
Starting Aspen... started.

Done, without errors.
[gittip] $ cat foo.log
Still here.
Still here.
Still here.
Still here.
Still here.
Still here.
Still here.
Killed!
[gittip] $

@chadwhitacre
Copy link
Contributor

Here's a demonstration of the problem:

$ ps ww|grep gunicorn
79870 s005  S+     0:00.00 grep gunicorn
$ ./node_modules/.bin/grunt aspen:start
Running "aspen:start" task
Starting Aspen... started.

Done, without errors.
$ ps ww|grep gunicorn
79873 s005  S      0:00.25 /Users/whit537/personal/gittip/www.gittip.com/env/bin/python2.7 ./env/bin/gunicorn aspen.wsgi:website
79876 s005  S      0:00.42 /Users/whit537/personal/gittip/www.gittip.com/env/bin/python2.7 ./env/bin/gunicorn aspen.wsgi:website
79879 s005  R+     0:00.00 grep gunicorn
$

@chadwhitacre
Copy link
Contributor

We're calling honcho from grunt. We are reasonably certain that honcho is receiving the SIGTERM, because we know that a stub server.py does. Also, honcho is gone! The problem seems to be that honcho does not propagate SIGTERM to children when it receives one itself.

@chadwhitacre
Copy link
Contributor

Relevant: nickstenning/honcho#32.

Honcho does not behave well when sent a SIGTERM. See:

nickstenning/honcho#32

I started down the rabbit hole of dusting off that PR, but ran out of
steam. This commit simply takes honcho out of the mix when calling grunt
test.
@chadwhitacre
Copy link
Contributor

Oh well. Worked on nickstenning/honcho#32 for a while and gave up.

Ready on this PR when you are, @Changaco.

Changaco added a commit that referenced this pull request Jul 9, 2014
@Changaco Changaco merged commit 99d1bfe into master Jul 9, 2014
@Changaco Changaco deleted the aspen-0.32 branch July 9, 2014 07:58
@Changaco
Copy link
Contributor Author

Changaco commented Jul 9, 2014

I tried to deploy this, it didn't work, so I looked for potential problems and made some changes, but it still doesn't work. The heroku error we're getting is H20 App boot timeout.

@Changaco
Copy link
Contributor Author

Changaco commented Jul 9, 2014

Problem solved. IRC.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants