Merge lp:~sakuag333/mailman/BUG_non_queue_runner into lp:mailman
Proposed by
Sandesh Kumar Agrawal
Status: | Merged |
---|---|
Approved by: | Barry Warsaw |
Approved revision: | 7207 |
Merge reported by: | Barry Warsaw |
Merged at revision: | not available |
Proposed branch: | lp:~sakuag333/mailman/BUG_non_queue_runner |
Merge into: | lp:mailman |
Diff against target: |
204 lines (+48/-13) 8 files modified
src/mailman/config/mailman.cfg (+2/-0) src/mailman/core/runner.py (+9/-3) src/mailman/core/switchboard.py (+13/-10) src/mailman/interfaces/runner.py (+4/-0) src/mailman/rest/tests/test_root.py (+7/-0) src/mailman/runners/lmtp.py (+5/-0) src/mailman/runners/rest.py (+1/-0) src/mailman/runners/tests/test_lmtp.py (+7/-0) |
To merge this branch: | bzr merge lp:~sakuag333/mailman/BUG_non_queue_runner |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw | Pending | ||
Review via email: mp+143640@code.launchpad.net |
To post a comment you must log in.
Hi Sandesh,
Thanks for taking another run at this bug. In general, I think you've
addressed the bug nicely, although there are a few things that still need
fixing. Since the branch is close enough, I'll just fix those when I merge
your branch, but I'll comment on the issues below for future reference.
These are all minor issues. Thanks for your contribution to Mailman!
I'll omit the diff blocks that look fine.
On Jan 17, 2013, at 07:45 AM, Sandesh Kumar Agrawal wrote:
=== modified file 'src/mailman/ core/runner. py' core/runner. py 2013-01-01 14:05:42 +0000 core/runner. py 2013-01-17 07:44:42 +0000 IRunner)
--- src/mailman/
+++ src/mailman/
> @@ -52,6 +52,7 @@
> @implementer(
> class Runner:
> intercept_signals = True
> + is_non_queue_runner = False
In general, I prefer positive flags rather than negative flags. This is
because it's usually more difficult to reason about double negatives. I'll
change the sense of this to "is_queue_runner = True" and then override the
flag to False in the lmtp and rest runners.
> 'name'] = name directory = expand( section. path, substitutions) instances) directory, slice, numslices, True)
> def __init__(self, name, slice=None):
> """Create a runner.
> @@ -66,10 +67,15 @@
> section = getattr(config, 'runner.' + name)
> substitutions = config.paths
> substitutions[
> - self.queue_
> numslices = int(section.
> - self.switchboard = Switchboard(
> - name, self.queue_
> + # Check whether the runner is queue runner or not; non queue runner should not have queue_directory or switchboard instance.
PEP 8 has a line length limit of 79 characters, so this line needs to be
wrapped.
> + if self.is_ non_queue_ runner: directory = None
> + self.queue_
> + self.switchboard= None
PEP 8 requires a single space both before and after the = sign.
> + else: directory = expand( section. path, substitutions) directory, slice, numslices, True) section. sleep_time) time.days +
> + self.queue_
> + self.switchboard = Switchboard(
> + name, self.queue_
> self.sleep_time = as_timedelta(
> # sleep_time is a timedelta; turn it into a float for time.sleep().
> self.sleep_float = (86400 * self.sleep_
=== modified file 'src/mailman/ core/switchboar d.py' core/switchboar d.py 2013-01-01 14:05:42 +0000 core/switchboar d.py 2013-01-17 07:44:42 +0000 numslices) ) directory = queue_directory queue_runner= {'lmtp' ,'rest' }
--- src/mailman/
+++ src/mailman/
> @@ -90,9 +90,10 @@
> 'Not a power of 2: {0}'.format(
> self.name = name
> self.queue_
> + self.non_
This should no longer be necessary, right? (I hope it's not! :)
> # If configured to, create the directory if it doesn't yet exist. create_ paths: self.queue_ directory, 0770) self.queue_ directory, 0770)
> if config.
> - makedirs(
> + makedirs(
This line should not be changed, and besides, PEP 8 requires a space after the
comma.
> # Fast track for no slices
> self._lower = None
> self._upper = None
> @@ -143,11 +144,11 @@
> ...