Merge lp:~sakuag333/mailman/non-queue-runner into lp:mailman

[Sandesh Kumar Agrawal]

I have made changes in switchboard.py
Now the __init()__ does not create directories var/queue/lmtp and var/queue/rest

[Barry Warsaw (barry)]

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
self.queue_directory and self.switchboard values (or more correctly, if the
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/switchboard.py'
--- src/mailman/core/switchboard.py 2013-01-01 14:05:42 +0000
+++ src/mailman/core/switchboard.py 2013-01-11 15:52:36 +0000
> @@ -90,9 +90,17 @@
> 'Not a power of 2: {0}'.format(numslices))
> self.name = name
> self.queue_directory = queue_directory
> + self.non_queue_runner={'lmtp','rest'}
> # If configured to, create the directory if it doesn't yet exist.
> if config.create_paths:
> - makedirs(self.queue_directory, 0770)
> + for directory in self.queue_directory.split():
> + is_non_queue_runner=False
> + for queue in self.non_queue_runner:
> + if queue in directory:
> + is_non_queue_runner=True
> + break
> + if not(is_non_queue_runner):
> + makedirs(directory,0770)

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
> 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(fp.fileno())
> - os.rename(tmpfile, filename)
> + fp.write(msgsave)
> + cPickle.dump(data, fp, protocol)
> + fp.flush()
> + os.fsync(fp.fileno())
> + 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!

[Barry Warsaw (barry)]

review: Needs Fixing

=== modified file 'src/mailman/core/switchboard.py'
--- src/mailman/core/switchboard.py	2013-01-01 14:05:42 +0000
+++ src/mailman/core/switchboard.py	2013-01-11 15:52:36 +0000
@@ -90,9 +90,17 @@
             'Not a power of 2: {0}'.format(numslices))
         self.name = name
         self.queue_directory = queue_directory
+        self.non_queue_runner={'lmtp','rest'}
         # If configured to, create the directory if it doesn't yet exist.
         if config.create_paths:
-            makedirs(self.queue_directory, 0770)
+            for directory in self.queue_directory.split():
+            	is_non_queue_runner=False
+            	for queue in self.non_queue_runner:
+            		if queue in directory:
+            			is_non_queue_runner=True
+            			break
+            	if not(is_non_queue_runner):
+            		makedirs(directory,0770)
         # Fast track for no slices
         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(fp.fileno())
-        os.rename(tmpfile, filename)
+          	fp.write(msgsave)
+            	cPickle.dump(data, fp, protocol)
+            	fp.flush()
+            	os.fsync(fp.fileno())
+       	os.rename(tmpfile, filename)
         return filebase
 
     def dequeue(self, filebase):