Launchpad itself

Merge lp:~gary/launchpad/bug662912 into lp:launchpad

  1. bug662912
  2. Merge into devel
Proposed by Gary Poster
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 11797
Proposed branch: lp:~gary/launchpad/bug662912
Merge into: lp:launchpad
Diff against target: 100 lines (+21/-2)
4 files modified
.bzrignore (+1/-0)
configs/development/launchpad-lazr.conf (+1/-0)
daemons/librarian.tac (+8/-2)
lib/canonical/librarian/web.py (+11/-0)
To merge this branch: bzr merge lp:~gary/launchpad/bug662912
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+39315@code.launchpad.net

Commit message

Add diagnostics for bug 662912

Description of the change

This branch adds diagnostics to approach bug 662912. As noted in my comment #1 for that bug, I have a hypothesis that this is caused by a rejected hostname. This branch adds log messages for that case, plus all other cases that generate a 404 other than a simple missing file (_eb_getFileAlias in c/librarian/web.py).

It also puts the message about upstream librarians in the logfile so it is easier to see that the configuration is as expected.

It also creates a log file destination for development so developers can look at the locally-generated logs.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

The development target for logs is unneeded and harmful, please don't
do that. My librarian branch already captures the log per-test and
includes in the test details.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

The test output won't change though will it? You need to change the testrunner config for that.

Everything else looks good although as I said on IRC, consider using addSystemEventTrigger instead of callWhenRunning, but that's optional.

review: Approve
Revision history for this message
Gary Poster (gary) wrote :

Thank you for the review and comments.

Robert approved the librarian change on IRC.

I looked at the addSystemEventTrigger approach. The behavior is identical to the callWhenRunning version. It's a bit longer spelling, but I guess it is a bit easier to understand the intent. That's a compelling argument, so I made the change.

Thanks again,

Gary

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2010-10-18 21:53:28 +0000
+++ .bzrignore 2010-10-25 22:09:50 +0000
@@ -80,3 +80,4 @@
80*.pt.py80*.pt.py
81.project81.project
82.pydevproject82.pydevproject
83librarian.log
8384
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2010-10-21 03:22:06 +0000
+++ configs/development/launchpad-lazr.conf 2010-10-25 22:09:50 +0000
@@ -175,6 +175,7 @@
175[librarian_server]175[librarian_server]
176root: /var/tmp/fatsam176root: /var/tmp/fatsam
177launch: True177launch: True
178logfile: librarian.log
178179
179[malone]180[malone]
180bugmail_error_from_address: noreply@bugs.launchpad.net181bugmail_error_from_address: noreply@bugs.launchpad.net
181182
=== modified file 'daemons/librarian.tac'
--- daemons/librarian.tac 2010-10-20 18:43:29 +0000
+++ daemons/librarian.tac 2010-10-25 22:09:50 +0000
@@ -9,6 +9,8 @@
9from meliae import scanner9from meliae import scanner
1010
11from twisted.application import service, strports11from twisted.application import service, strports
12from twisted.internet import reactor
13from twisted.python import log
12from twisted.web import server14from twisted.web import server
1315
14from canonical.config import config, dbconfig16from canonical.config import config, dbconfig
@@ -29,10 +31,14 @@
29if config.librarian_server.upstream_host:31if config.librarian_server.upstream_host:
30 upstreamHost = config.librarian_server.upstream_host32 upstreamHost = config.librarian_server.upstream_host
31 upstreamPort = config.librarian_server.upstream_port33 upstreamPort = config.librarian_server.upstream_port
32 print 'Using upstream librarian http://%s:%d' % (34 reactor.addSystemEventTrigger(
33 upstreamHost, upstreamPort)35 'before', 'startup', log.msg,
36 'Using upstream librarian http://%s:%d' %
37 (upstreamHost, upstreamPort))
34else:38else:
35 upstreamHost = upstreamPort = None39 upstreamHost = upstreamPort = None
40 reactor.addSystemEventTrigger(
41 'before', 'startup', log.msg, 'Not using upstream librarian')
3642
37application = service.Application('Librarian')43application = service.Application('Librarian')
38librarianService = service.IServiceCollection(application)44librarianService = service.IServiceCollection(application)
3945
=== modified file 'lib/canonical/librarian/web.py'
--- lib/canonical/librarian/web.py 2010-09-24 15:40:49 +0000
+++ lib/canonical/librarian/web.py 2010-10-25 22:09:50 +0000
@@ -7,6 +7,7 @@
7import time7import time
8from urlparse import urlparse8from urlparse import urlparse
99
10from twisted.python import log
10from twisted.web import resource, static, util, server, proxy11from twisted.web import resource, static, util, server, proxy
11from twisted.internet.threads import deferToThread12from twisted.internet.threads import deferToThread
1213
@@ -52,6 +53,8 @@
52 try:53 try:
53 aliasID = int(name)54 aliasID = int(name)
54 except ValueError:55 except ValueError:
56 log.msg(
57 "404: alias is not an int: %r" % (name,))
55 return fourOhFour58 return fourOhFour
5659
57 return LibraryFileAliasResource(self.storage, aliasID,60 return LibraryFileAliasResource(self.storage, aliasID,
@@ -76,6 +79,8 @@
76 try:79 try:
77 self.aliasID = int(filename)80 self.aliasID = int(filename)
78 except ValueError:81 except ValueError:
82 log.msg(
83 "404 (old URL): alias is not an int: %r" % (name,))
79 return fourOhFour84 return fourOhFour
80 filename = request.postpath[0]85 filename = request.postpath[0]
8186
@@ -95,6 +100,9 @@
95 netloc = netloc[:netloc.find(':')]100 netloc = netloc[:netloc.find(':')]
96 expected_hostname = 'i%d.restricted.%s' % (self.aliasID, netloc)101 expected_hostname = 'i%d.restricted.%s' % (self.aliasID, netloc)
97 if expected_hostname != hostname:102 if expected_hostname != hostname:
103 log.msg(
104 '404: expected_hostname != hostname: %r != %r' %
105 (expected_hostname, hostname))
98 return fourOhFour106 return fourOhFour
99107
100 token = request.args.get('token', [None])[0]108 token = request.args.get('token', [None])[0]
@@ -128,6 +136,9 @@
128 # a crude form of access control (stuff we care about can have136 # a crude form of access control (stuff we care about can have
129 # unguessable names effectively using the filename as a secret).137 # unguessable names effectively using the filename as a secret).
130 if dbfilename.encode('utf-8') != filename:138 if dbfilename.encode('utf-8') != filename:
139 log.msg(
140 "404: dbfilename.encode('utf-8') != filename: %r != %r"
141 % (dbfilename.encode('utf-8'), filename))
131 return fourOhFour142 return fourOhFour
132143
133 if not restricted:144 if not restricted: