Merge lp:~sakuag333/mailman/non-queue-runner into lp:mailman
Proposed by
Sandesh Kumar Agrawal
Status: | Merged |
---|---|
Merged at revision: | 7202 |
Proposed branch: | lp:~sakuag333/mailman/non-queue-runner |
Merge into: | lp:mailman |
Diff against target: |
39 lines (+14/-6) 1 file modified
src/mailman/core/switchboard.py (+14/-6) |
To merge this branch: | bzr merge lp:~sakuag333/mailman/non-queue-runner |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw | Needs Fixing | ||
Review via email:
|
Description of the change
I have made changes in switchboard.py
Now the __init()__ does not create directories var/queue/lmtp and var/queue/rest
To post a comment you must log in.
Thanks for the branch Sandesh. Here's a review of the code. I think there's
a better way of inhibiting the creation of the queue directory that doesn't
involve hard coding the non-queue runners in the Switchboard constructor. The
problem with that is that if we ever add another non-queue runner, we'll have
to change the hard coded value.
The way I think this should be done is by adding another boolean attribute in
the Runner base class called `is_queue_runner`. This would be set to True by
default. The LMTPRunner and RESTRunner would set this attribute to False in
their class definitions, but all the other classes would just use the base
class value.
Then, in Runner.__init__(), you'd check this flag before creating the directory and self.switchboard values (or more correctly, if the
self.queue_
class attribute is False, you'd set these both to None).
A few other general comments.
I generally don't like to commit code changes that don't also come with tests.
I think this test should be pretty easy, but I am happy to write the test if
you can't, don't want to. If you'd like to try that but are unsure about how
or where to do it, let me know and I'll help.
Let's look a the code.
=== 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-11 15:52:36 +0000 numslices) ) directory = queue_directory queue_runner= {'lmtp' ,'rest' } create_ paths: self.queue_ directory, 0770) directory. split() : queue_runner= False queue_runner: queue_runner= True non_queue_ runner) : directory, 0770)
--- src/mailman/
+++ src/mailman/
> @@ -90,9 +90,17 @@
> 'Not a power of 2: {0}'.format(
> self.name = name
> self.queue_
> + self.non_
> # If configured to, create the directory if it doesn't yet exist.
> if config.
> - makedirs(
> + for directory in self.queue_
> + is_non_
> + for queue in self.non_
> + if queue in directory:
> + is_non_
> + break
> + if not(is_
> + makedirs(
Please be very careful to only uses spaces and not tabs. We don't allow tab
indentation in the Mailman code base. Most editors have a way to set this. (I
can help with Emacs, but you're probably on your own for other editors. ;).
> # Fast track for no slices fp.fileno( )) fp.fileno( ))
> self._lower = None
> self._upper = None
> @@ -143,11 +151,11 @@
> data['_parsemsg'] = (protocol == 0)
> # Write to the pickle file the message object and metadata.
> with open(tmpfile, 'w') as fp:
> - fp.write(msgsave)
> - cPickle.dump(data, fp, protocol)
> - fp.flush()
> - os.fsync(
> - os.rename(tmpfile, filename)
> + fp.write(msgsave)
> + cPickle.dump(data, fp, protocol)
> + fp.flush()
> + os.fsync(
> + os.rename(tmpfile, filename)
> return filebase
>
> def dequeue(self, filebase):
Please try again. You can just push an update when you're ready. And again,
thanks very much for your contribution to Mailman!