Merge lp:~benji/launchpad/bug-622765 into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Brad Crittenden | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 11468 | ||||
Proposed branch: | lp:~benji/launchpad/bug-622765 | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
125 lines (+26/-16) 3 files modified
lib/lp/services/apachelogparser/base.py (+2/-4) lib/lp/services/apachelogparser/script.py (+1/-1) lib/lp/services/apachelogparser/tests/test_apachelogparser.py (+23/-11) |
||||
To merge this branch: | bzr merge lp:~benji/launchpad/bug-622765 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+33951@code.launchpad.net |
Description of the change
When parsing Apache log files, the cron scripts want to pick up where
they left off. The lp.services.
function used to return a mapping of open files to the position at which
parsing should resume. The problem was that all the files (which could
number in the thousands) were opened at once and returned en masse.
Since there was only one caller of get_files_to_parse and it only
iterated over the .items() of the result, it was easy to refactor the
code to instead return/consume a generator. The result is that only one
file is open at any given time (the client consumes each file and closes
it before proceeding to the next).
Gary and I had a pre-implementation call.
The get_files_to_parse function was already well tested and only trivial
transformations were required to make the tests compatible with the new
API.
The tests can be run with the command:
bin/test -c test_apachelogp
I verified that the scripts still run by faking up enough of a
production environment that they would at least run to completion
without generating errors.
No lint was reported by "make lint".
Nice branch Benji. Neat to be able to reduce resource usage so easily.