launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01760
[Merge] lp:~wallyworld/launchpad/log-files-in-subdir into lp:launchpad/devel
Ian Booth has proposed merging lp:~wallyworld/launchpad/log-files-in-subdir into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
= Summary =
A bit log log file cleanup - the launchpad log and trace files are put in a logs subdirectory instead of the project root directory. This gives a cleaner project directory, allows easier management of the files, a more robust .bzrignore entry (no need to enumerate all possible log files, just a single "log" entry will do), and it allows IDE's to be configured to ignore log files, saving lots of CPU and parsing overhead overhead.
= Implementation =
Consideration was given to using a config setting for log_folder but the zope logger configs only allow a hard coded path eg logs/launchpad.log so it was decided just to use a hard coded logs directory in other places too. The affected log files include those associated with Launchpad itself:
- thread*.request
- launchpad.log
- launchpad-access.log
- trace.log
- pagetests-access.log
- google-stub.log
Other log files for applications like loggerhead or the ssh server etc were not changed.
*Question*
I need guidance on what cron or other clean/garbo up scripts may be affected. I changed everything I know about and a development system and tests run ok but I'm not sure about production deployment issues.
= Tests =
bin/test -vvt xx-pagetest-logging.txt
Other regression tests
make run
= Launchpad lint =
The lint issues are existing
Checking for conflicts and issues in changed files.
Linting changed files:
.bzrignore
Makefile
configs/development/launchpad.conf
configs/test-playground/launchpad.conf
configs/testrunner/launchpad.conf
configs/testrunner-appserver/launchpad.conf
lib/canonical/config/schema-lazr.conf
lib/canonical/launchpad/pagetests/standalone/xx-pagetest-logging.txt
lib/canonical/launchpad/scripts/runlaunchpad.py
lib/canonical/launchpad/webapp/publication.py
lib/canonical/testing/layers.py
./Makefile
63: Line exceeds 78 characters.
71: Line exceeds 78 characters.
136: Line exceeds 78 characters.
146: Line exceeds 78 characters.
180: Line exceeds 78 characters.
242: Line exceeds 78 characters.
368: Line exceeds 78 characters.
407: Line exceeds 78 characters.
408: Line exceeds 78 characters.
433: Line exceeds 78 characters.
434: Line exceeds 78 characters.
./lib/canonical/config/schema-lazr.conf
515: Line exceeds 78 characters.
1013: Line exceeds 78 characters.
./lib/canonical/launchpad/pagetests/standalone/xx-pagetest-logging.txt
1: narrative uses a moin header.
./lib/canonical/launchpad/scripts/runlaunchpad.py
42: W291 trailing whitespace
324: E501 line too long (82 characters)
325: E501 line too long (80 characters)
42: Line has trailing whitespace.
323: Line exceeds 78 characters.
324: Line exceeds 78 characters.
325: Line exceeds 78 characters.
./lib/canonical/launchpad/webapp/publication.py
459: Line exceeds 78 characters.
469: Line exceeds 78 characters.
470: Line exceeds 78 characters.
./lib/canonical/testing/layers.py
86: redefinition of unused 'zope' from line 85
577: redefinition of unused 'pidfile' from line 99
60: 'shutil' imported but unused
382: E231 missing whitespace after ','
451: E202 whitespace before ')'
637: E202 whitespace before ')'
653: E202 whitespace before ')'
673: E202 whitespace before ')'
897: E301 expected 1 blank line, found 0
906: W291 trailing whitespace
1038: E301 expected 1 blank line, found 2
1065: E202 whitespace before ')'
1075: E202 whitespace before ')'
1121: E202 whitespace before ')'
1142: E202 whitespace before ')'
1180: E301 expected 1 blank line, found 2
1277: E301 expected 1 blank line, found 0
1340: E302 expected 2 blank lines, found 3
1443: E202 whitespace before ')'
1456: E202 whitespace before ')'
1460: E301 expected 1 blank line, found 2
1544: E301 expected 1 blank line, found 0
1564: E301 expected 1 blank line, found 0
1594: E301 expected 1 blank line, found 2
1652: E301 expected 1 blank line, found 0
906: Line has trailing whitespace.
911: Line exceeds 78 characters.
1826: Line exceeds 78 characters.
Process finished with exit code 0
--
https://code.launchpad.net/~wallyworld/launchpad/log-files-in-subdir/+merge/39814
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/log-files-in-subdir into lp:launchpad/devel.
=== modified file '.bzrignore'
--- .bzrignore 2010-10-25 20:42:59 +0000
+++ .bzrignore 2010-11-02 01:56:24 +0000
@@ -1,9 +1,6 @@
.tags
bzr-version-info.py
-launchpad.log
-google-stub.log
-launchpad-access.log
-pagetests-access.log
+logs
+*
sourcecode/*
./botmaster
@@ -20,9 +17,6 @@
lint.txt
.shelf
development-nohup.out
-testrunner-appserver-quiet.log
-test-appserver-layer.log
-google-stub.log
opensource/launchpadlib/build
opensource/launchpadlib/dist
opensource/launchpadlib/*.egg*
@@ -36,8 +30,6 @@
lib/canonical/launchpad/apidoc/wadl-test-playground.xml
lib/canonical/launchpad/icing/build/*
lib/canonical/launchpad/icing/combo.css
-trace.log
-test-appserver-layer-trace.log
bin
develop-eggs
.installed.cfg
=== modified file 'Makefile'
--- Makefile 2010-10-21 03:22:06 +0000
+++ Makefile 2010-11-02 01:56:24 +0000
@@ -228,17 +228,17 @@
$(PY) cronscripts/merge-proposal-jobs.py -v
run: check_schema inplace stop
- $(RM) thread*.request
+ $(RM) logs/thread*.request
bin/run -r librarian,google-webservice,memcached -i $(LPCONFIG)
start-gdb: check_schema inplace stop support_files
- $(RM) thread*.request
+ $(RM) logs/thread*.request
nohup gdb -x run.gdb --args bin/run -i $(LPCONFIG) \
-r librarian,google-webservice
> ${LPCONFIG}-nohup.out 2>&1 &
run_all: check_schema inplace stop
- $(RM) thread*.request
+ $(RM) logs/thread*.request
bin/run -r librarian,sftp,forker,mailman,codebrowse,google-webservice,memcached \
-i $(LPCONFIG)
@@ -252,7 +252,7 @@
$(PY) scripts/stop-loggerhead.py
run_codehosting: check_schema inplace stop
- $(RM) thread*.request
+ $(RM) logs/thread*.request
bin/run -r librarian,sftp,forker,codebrowse -i $(LPCONFIG)
start_librarian: compile
@@ -342,7 +342,7 @@
-name '*.lo' -o -name '*.py[co]' -o -name '*.dll' -o \
-name '*.pt.py' \) \
-print0 | xargs -r0 $(RM)
- $(RM) thread*.request
+ $(RM) logs/thread*.request
$(RM) -r lib/mailman
$(RM) -rf lib/canonical/launchpad/icing/build/*
$(RM) -r $(CODEHOSTING_ROOT)
=== modified file 'configs/development/launchpad.conf'
--- configs/development/launchpad.conf 2010-07-16 20:32:02 +0000
+++ configs/development/launchpad.conf 2010-11-02 01:56:24 +0000
@@ -43,7 +43,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad-access.log
+ path logs/launchpad-access.log
</logfile>
<logfile>
@@ -57,7 +57,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad.log
+ path logs/launchpad.log
</logfile>
<logfile>
@@ -71,7 +71,7 @@
<logfile>
format %(message)s
- path trace.log
+ path logs/trace.log
</logfile>
</logger>
=== modified file 'configs/test-playground/launchpad.conf'
--- configs/test-playground/launchpad.conf 2009-09-11 18:14:50 +0000
+++ configs/test-playground/launchpad.conf 2010-11-02 01:56:24 +0000
@@ -43,7 +43,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad-access.log
+ path logs/launchpad-access.log
</logfile>
<logfile>
@@ -57,7 +57,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad.log
+ path logs/launchpad.log
</logfile>
<logfile>
@@ -71,7 +71,7 @@
<logfile>
format %(message)s
- path trace.log
+ path logs/trace.log
</logfile>
</logger>
=== modified file 'configs/testrunner-appserver/launchpad.conf'
--- configs/testrunner-appserver/launchpad.conf 2009-09-11 18:14:50 +0000
+++ configs/testrunner-appserver/launchpad.conf 2010-11-02 01:56:24 +0000
@@ -28,13 +28,13 @@
<accesslog>
<logfile>
- path test-appserver-layer.log
+ path logs/test-appserver-layer.log
</logfile>
</accesslog>
<eventlog>
<logfile>
- path test-appserver-layer.log
+ path logs/test-appserver-layer.log
</logfile>
</eventlog>
@@ -44,7 +44,7 @@
<logfile>
format %(message)s
- path test-appserver-layer-trace.log
+ path logs/test-appserver-layer-trace.log
</logfile>
</logger>
=== modified file 'configs/testrunner/launchpad.conf'
--- configs/testrunner/launchpad.conf 2008-10-17 18:39:23 +0000
+++ configs/testrunner/launchpad.conf 2010-11-02 01:56:24 +0000
@@ -43,7 +43,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad-access.log
+ path logs/launchpad-access.log
</logfile>
<logfile>
@@ -57,7 +57,7 @@
# filesystem path or the tokens STDOUT or STDERR.
<logfile>
- path launchpad.log
+ path logs/launchpad.log
</logfile>
<logfile>
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2010-10-29 02:10:17 +0000
+++ lib/canonical/config/schema-lazr.conf 2010-11-02 01:56:24 +0000
@@ -884,7 +884,7 @@
mapfile: lib/canonical/launchpad/ftests/googlesearches/mapping.txt
# Where should the service log files live?
-log: google-stub.log
+log: logs/google-stub.log
# Do we actually want to run the service?
launch: False
=== modified file 'lib/canonical/launchpad/pagetests/standalone/xx-pagetest-logging.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-pagetest-logging.txt 2008-01-17 20:11:29 +0000
+++ lib/canonical/launchpad/pagetests/standalone/xx-pagetest-logging.txt 2010-11-02 01:56:24 +0000
@@ -5,7 +5,7 @@
>>> browser.open('http://launchpad.dev/')
- >>> log = open('pagetests-access.log').read()
+ >>> log = open('logs/pagetests-access.log').read()
>>> print log.strip().split('\n')[-1]
127.0.0.88 - ... "launchpad.dev" [...] "GET / HTTP/1.1" 200 ...
"Anonymous" "RootObject:index.html" "localhost" "Python-urllib/..."
=== modified file 'lib/canonical/launchpad/scripts/runlaunchpad.py'
--- lib/canonical/launchpad/scripts/runlaunchpad.py 2010-10-14 20:41:13 +0000
+++ lib/canonical/launchpad/scripts/runlaunchpad.py 2010-11-02 01:56:24 +0000
@@ -312,6 +312,11 @@
# Store our process id somewhere
make_pidfile('launchpad')
+ # Ensure our logs directory exists
+ log_folder = 'logs'
+ if not os.path.exists(log_folder):
+ os.mkdir(log_folder)
+
if config.launchpad.launch:
main(argv)
else:
=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py 2010-10-02 11:11:49 +0000
+++ lib/canonical/launchpad/webapp/publication.py 2010-11-02 01:56:24 +0000
@@ -245,7 +245,7 @@
notify(StartRequestEvent(request))
request._traversalticks_start = tickcount.tickcount()
threadid = thread.get_ident()
- threadrequestfile = open('thread-%s.request' % threadid, 'w')
+ threadrequestfile = open('logs/thread-%s.request' % threadid, 'w')
try:
request_txt = unicode(request).encode('UTF-8')
except Exception:
=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py 2010-10-27 04:23:52 +0000
+++ lib/canonical/testing/layers.py 2010-11-02 01:56:24 +0000
@@ -275,6 +275,10 @@
@profiled
def setUp(cls):
BaseLayer.isSetUp = True
+ # Ensure our logs directory exists
+ log_folder = 'logs'
+ if not os.path.exists(log_folder):
+ os.mkdir(log_folder)
cls.fixture = Fixture()
cls.fixture.setUp()
cls.fixture.addCleanup(setattr, cls, 'fixture', None)
@@ -1519,7 +1523,7 @@
PageTestLayer.profiler = Profile()
else:
PageTestLayer.profiler = None
- file_handler = logging.FileHandler('pagetests-access.log', 'w')
+ file_handler = logging.FileHandler('logs/pagetests-access.log', 'w')
file_handler.setFormatter(logging.Formatter())
logger = PythonLogger('pagetests-access')
logger.logger.addHandler(file_handler)
Follow ups
