Merge lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development

[kanika vats]

[Robin J (robin-jeffries)]

I'm learning how to do this. I will put comments below in the diff; let me
know if that is readable.
On thing I would like you to do is limit the line length, at least for new
lines you add, to 80 chars. You should be able to tell from the diff format
which lines have wrapped (in your changes). If you need help in figuring
out how best to do continuation lines, let me know (in my experience, you
sometimes have to wrap expressions in parens to make it work well -- at
least if you are using emacs and letting it do the formatting).

I have a few questions, that are as much for me to learn the code as
anything. There are 5 comments below, if I counted right. One I think is a
bug and the rest are style comments or questions. I will review your next
changes as soon as I get them.

review needsfixing

On Wed, May 26, 2010 at 11:45 AM, kanika vats <email address hidden>wrote:

> kanika vats has proposed merging lp:~systers-soc/systers/stable into
> lp:~systers-dev/systers/development.
>
> Requested reviews:
> Kari Tornow (kari-tornow)
> Robin J (robin-jeffries)
>
> --
> https://code.launchpad.net/~systers-soc/systers/stable/+merge/26090<https://code.launchpad.net/%7Esysters-soc/systers/stable/+merge/26090>
> You are requested to review the proposed merge of
> lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development.
>
> === modified file 'Mailman/DlistUtils.py'
> --- Mailman/DlistUtils.py 2010-01-31 20:29:48 +0000
> +++ Mailman/DlistUtils.py 2010-05-26 18:45:43 +0000
> @@ -76,11 +76,11 @@
> if addr == None:
> return None
>
> - command = "result =
> self.store.find(Subscriber,Subscriber.mailman_key ==
> unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for
> subscriber in result]\n"
> + command = "result =
> self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for
> subscriber in result]\n"
> if DEBUG_MODE:
> syslog('info', "DlistUtils:(getSubscriber_id_raw)executing
> query:\n%s", command)
> #storm recognizes unicode only
> - result = self.store.find(Subscriber,Subscriber.mailman_key ==
> unicode(addr.lower(),"utf-8"))
> + result = self.store.find(Subscriber,Subscriber.mailman_key.lower()
> == unicode(addr,"utf-8"))
> a = [(subscriber.subscriber_id) for subscriber in result]
> if DEBUG_MODE:
> syslog('info', 'value of a is: %s\n', a)
> @@ -136,10 +136,10 @@
> @decfunc
> def setDisable(self, member, flag):
> """Disable/enable delivery based on mm_cfg.DisableDelivery"""
> - command = "result =
> self.store.find(Subscriber,Subscriber.mailman_key ==
> unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in
> result]\n"
> + command = "result =
> self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in
> result]\n"
> if DEBUG_MODE:
> syslog('info', 'DlistUtils(setDisable):Executing query:\n%s\n
> Member whoes suppress value is to be found \n %s', command,member)
>

can you put a spa...

[kanika vats (kanika-krikan)]

> can you put a space in there
> command, member
> (a review is as much about style as substance)
> whoes => whose

Done

>
> Why is the commit not the last database command in the function? Actually,
> the other functions make changes, but they don't have a commit. What's the
> role of the commit?

Hmm..(I had previously explained the use of decorator function which is to wrap both the starting and ending of every function that is decorated with some common starting lines and ending lines)If you closely see decorator function at the top of DlistUtils you will see after

'result = f(self, *args)'->This executes all the commands defined in the function that calls the decorator function

you have

'self.store.commit()' ->Thus,this line is always executed after all the commands in the decorated function
                             are executed.

Thus the other functions make changes but they dont define commit explicitly because the decorator function automatically calls 'self.store.commit() after the complete execution of that function(every function that has a "@decfunc" mentioned just before the start of function defination is decorated)

Now what is the role of commit??
->Whenever you call store functions in Storm to make changes in the database for eg to change the value of a particular attribute or to add new tuple or delete one etc then Storm calculates the changes but does not reflects them in the database until and unless you do a commit.

Now why the above function related to your question had a commit mentioned in it although it was decorated and decorator function would have committed anyway?
->The answer can be easily understood if we closely examine the store functions being used.
The first command is:

1)self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8"))

This is setting the mailman_key to newaddr but like I said that this change wont get reflected to database without a commit.

Now the socond command executed after this is:

2)self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8")).set(deleted = False)

Its finding the database of the tuples which have mailman_key == newaddr and setting deleted to False.If I had not given a commit before, then this query would not have given correct results as the previous queries action would not have been reflected in the database.

I hope this explains everything regarding why the commit had to be explicitly used in the above function.

>
> So Email is a list, right? Why is it going to match the key (which I assume
> is a string)? And what if there are two people with that key? I realize
> that is an error condition, but what's going to happen?
>

Absolutely I added the above lines of code to make sure that If a person who was previously a member of the list with the email address in lowercase, now subscribes back again but this time preferring uppercase for the email name then the database does not gets corrupted and changes the subscriber.mailman_key to the newaddr(uppercase one) preferred.

I have added Email = Email[0] to remove the fault.

>
> in ...

[kanika vats (kanika-krikan)]

Also I have very less knowledge on how to do a merge and code review done.After committing the change will I have to resubmit the proposal or the present merge proposal will take into account new changes as well?

[Jennifer Redman (jenred)]

Do not commit to stable commit to dev.

Jen

On May 27, 2010, at 10:42, kanika vats <email address hidden> wrote:

>
>> can you put a space in there
>> command, member
>> (a review is as much about style as substance)
>> whoes => whose
>
> Done
>
>>
>> Why is the commit not the last database command in the function?
>> Actually,
>> the other functions make changes, but they don't have a commit.
>> What's the
>> role of the commit?
>
> Hmm..(I had previously explained the use of decorator function which
> is to wrap both the starting and ending of every function that is
> decorated with some common starting lines and ending lines)If you
> closely see decorator function at the top of DlistUtils you will see
> after
>
> 'result = f(self, *args)'->This executes all the commands defined in
> the function that calls the decorator function
>
> you have
>
> 'self.store.commit()' ->Thus,this line is always executed after
> all the commands in the decorated function
> are executed.
>
> Thus the other functions make changes but they dont define commit
> explicitly because the decorator function automatically calls
> 'self.store.commit() after the complete execution of that function
> (every function that has a "@decfunc" mentioned just before the
> start of function defination is decorated)
>
> Now what is the role of commit??
> ->Whenever you call store functions in Storm to make changes in the
> database for eg to change the value of a particular attribute or to
> add new tuple or delete one etc then Storm calculates the changes
> but does not reflects them in the database until and unless you do a
> commit.
>
> Now why the above function related to your question had a commit
> mentioned in it although it was decorated and decorator function
> would have committed anyway?
> ->The answer can be easily understood if we closely examine the
> store functions being used.
> The first command is:
>
> 1)self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8"))
>
> This is setting the mailman_key to newaddr but like I said that this
> change wont get reflected to database without a commit.
>
> Now the socond command executed after this is:
>
> 2)self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> unicode(newaddr.lower(),"utf-8")).set(deleted = False)
>
> Its finding the database of the tuples which have mailman_key ==
> newaddr and setting deleted to False.If I had not given a commit
> before, then this query would not have given correct results as the
> previous queries action would not have been reflected in the database.
>
> I hope this explains everything regarding why the commit had to be
> explicitly used in the above function.
>
>
>>
>> So Email is a list, right? Why is it going to match the key (which
>> I assume
>> is a string)? And what if there are two people with that key? I
>> realize
>> that is an error condition, but what's going to happen?
>>
>
> Absolutely I added the above lines of code to make sure that If a
> person who was previously a member of the list ...

[kanika vats (kanika-krikan)]

Hi Jen,
I can not commit to the development branch as I see that I do not have
permissions.It says that only members of Systers can do it.If you give me
permissions that I will make a commit to it.
Thanks

On Thu, May 27, 2010 at 11:15 PM, Jennifer Redman <email address hidden> wrote:

> Do not commit to stable commit to dev.
>
> Jen
>
>
>
> On May 27, 2010, at 10:42, kanika vats <email address hidden> wrote:
>
> >
> >> can you put a space in there
> >> command, member
> >> (a review is as much about style as substance)
> >> whoes => whose
> >
> > Done
> >
> >>
> >> Why is the commit not the last database command in the function?
> >> Actually,
> >> the other functions make changes, but they don't have a commit.
> >> What's the
> >> role of the commit?
> >
> > Hmm..(I had previously explained the use of decorator function which
> > is to wrap both the starting and ending of every function that is
> > decorated with some common starting lines and ending lines)If you
> > closely see decorator function at the top of DlistUtils you will see
> > after
> >
> > 'result = f(self, *args)'->This executes all the commands defined in
> > the function that calls the decorator function
> >
> > you have
> >
> > 'self.store.commit()' ->Thus,this line is always executed after
> > all the commands in the decorated function
> > are executed.
> >
> > Thus the other functions make changes but they dont define commit
> > explicitly because the decorator function automatically calls
> > 'self.store.commit() after the complete execution of that function
> > (every function that has a "@decfunc" mentioned just before the
> > start of function defination is decorated)
> >
> > Now what is the role of commit??
> > ->Whenever you call store functions in Storm to make changes in the
> > database for eg to change the value of a particular attribute or to
> > add new tuple or delete one etc then Storm calculates the changes
> > but does not reflects them in the database until and unless you do a
> > commit.
> >
> > Now why the above function related to your question had a commit
> > mentioned in it although it was decorated and decorator function
> > would have committed anyway?
> > ->The answer can be easily understood if we closely examine the
> > store functions being used.
> > The first command is:
> >
> > 1)self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> > unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8"))
> >
> > This is setting the mailman_key to newaddr but like I said that this
> > change wont get reflected to database without a commit.
> >
> > Now the socond command executed after this is:
> >
> > 2)self.store.find(Subscriber,Subscriber.mailman_key.lower() ==
> > unicode(newaddr.lower(),"utf-8")).set(deleted = False)
> >
> > Its finding the database of the tuples which have mailman_key ==
> > newaddr and setting deleted to False.If I had not given a commit
> > before, then this query would not have given correct results as the
> > previous queries action would not have been reflected in the database.
> >
> > I hope this explains everything regarding why the commit had to be
> > explicitly used in...

[Robin J (robin-jeffries)]

review needsfixing

Trying to figure out how to make this work correctly.

Robin

On Wed, May 26, 2010 at 10:00 PM, Robin J <email address hidden> wrote:

> I'm learning how to do this. I will put comments below in the diff; let me
> know if that is readable.
>
> > kanika vats has proposed merging lp:~systers-soc/systers/stable into
> > lp:~systers-dev/systers/development.
> >
> > Requested reviews:
> > Kari Tornow (kari-tornow)
> > Robin J (robin-jeffries)
> >
> > --
> > https://code.launchpad.net/~systers-soc/systers/stable/+merge/26090<https://code.launchpad.net/%7Esysters-soc/systers/stable/+merge/26090>
> <https://code.launchpad.net/%7Esysters-soc/systers/stable/+merge/26090>
> > You are requested to review the proposed merge of
> > lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development.
> >
> > === modified file 'Mailman/DlistUtils.py'
> > --- Mailman/DlistUtils.py 2010-01-31 20:29:48 +0000
> > +++ Mailman/DlistUtils.py 2010-05-26 18:45:43 +0000
> > @@ -76,11 +76,11 @@
> > if addr == None:
> > return None
>

[Robin J (robin-jeffries)]

Yeah, apparently it doesn't take them into account. I see a link in the
upper right that says "Resubmit proposal". Why don't you see what that
does? (I still haven't figured out why it won't make sense of my review)

Robin

On Thu, May 27, 2010 at 10:45 AM, kanika vats <email address hidden>wrote:

> Also I have very less knowledge on how to do a merge and code review
> done.After committing the change will I have to resubmit the proposal or the
> present merge proposal will take into account new changes as well?
> --
> https://code.launchpad.net/~systers-soc/systers/stable/+merge/26090
> You are requested to review the proposed merge of
> lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development.
>

[Robin J (robin-jeffries)]

OK, I did this on the web, not via email. Let's see if it takes.

I'm happy with all the changes. We are ready to go, but
   - Kari, can you approve this ASAP. I want you to get the experience of seeing how a code review goes.
   - Jen, what should Kanika Vats do next? does she need to delete this proposal and remerge to a different branch? Is there a way to reparent this?

review: Approve

[Kari Tornow (kari-tornow)]

I am glad to see Robin give comments on the aesthetics of the code. I feel that is important. I hope to review in depth on the next go round.

[Kari Tornow (kari-tornow)]

Comments added in prior notes.

review: Approve

[Kari Tornow (kari-tornow)]

Approved via web. Seemed to work.

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Robin J
Sent: Thursday, May 27, 2010 11:09 PM
To: <email address hidden>
Subject: Re: [Merge] lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development

Review: Approve
OK, I did this on the web, not via email. Let's see if it takes.

I'm happy with all the changes. We are ready to go, but
   - Kari, can you approve this ASAP. I want you to get the experience of seeing how a code review goes.
   - Jen, what should Kanika Vats do next? does she need to delete this proposal and remerge to a different branch? Is there a way to reparent this?
--
https://code.launchpad.net/~systers-soc/systers/stable/+merge/26266
You are requested to review the proposed merge of lp:~systers-soc/systers/stable into lp:~systers-dev/systers/development.

This communication is the property of Qwest and may contain confidential or
privileged information. Unauthorized use of this communication is strictly
prohibited and may be unlawful. If you have received this communication
in error, please immediately notify the sender by reply e-mail and destroy
all copies of the communication and any attachments.

[Kari Tornow (kari-tornow)]

Approve without review.

review: Approve

=== modified file 'Mailman/DlistUtils.py'
--- Mailman/DlistUtils.py	2010-01-31 20:29:48 +0000
+++ Mailman/DlistUtils.py	2010-05-28 04:55:39 +0000
@@ -40,17 +40,17 @@
         if self == classObject:
             if self.store == None:#for the case when same object is used for calling more than one functions
                 self.store = Store(self.database)
-                result = f(self,*args)
+                result = f(self, *args)
                 self.store.commit()
                 self.store.close()
                 self.store = None
             else:#To handle the nesting issue
-                result = f(self,*args)
+                result = f(self, *args)
                 self.store.commit()
         else:#for the case whenever a new object calls its decorated member function
             classObject = self
             self.store = Store(self.database)
-            result = f(self,*args)
+            result = f(self, *args)
             self.store.commit()
             self.store.close()
             self.store = None
@@ -60,14 +60,14 @@
 #Define all the classes corresponding to the tables in the database
 class Subscriber(object):
     __storm_table__ = "subscriber"
-    subscriber_id = Int(primary = True,default = AutoReload)
+    subscriber_id = Int(primary = True, default = AutoReload)
     mailman_key = Unicode()
     preference = Int(default = 1)
     format = Int(default = 3)
     deleted = Bool(default = False)
     suppress = Int(default = 0)
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -76,11 +76,11 @@
         if addr == None:
             return None
 
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(addr.lower(),'utf-8'))\na = [(subscriber.subscriber_id) for subscriber in result]\n"
         if DEBUG_MODE:
             syslog('info', "DlistUtils:(getSubscriber_id_raw)executing query:\n%s", command)
         #storm recognizes unicode only
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(addr.lower(),"utf-8")) 
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(addr,"utf-8")) 
         a = [(subscriber.subscriber_id) for subscriber in result]
         if DEBUG_MODE:
             syslog('info', 'value of a is: %s\n', a)
@@ -136,70 +136,70 @@
     @decfunc
     def setDisable(self, member, flag):
         """Disable/enable delivery based on mm_cfg.DisableDelivery"""
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
         if DEBUG_MODE:
-            syslog('info', 'DlistUtils(setDisable):Executing query:\n%s\n Member whoes suppress value is to be found \n %s', command,member)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8"))
+            syslog('info', 'DlistUtils(setDisable):Executing query:\n%s\n Member whose suppress value is to be found \n %s', command, member)
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8"))
         oldval = [(subscriber.suppress) for subscriber in result]
 	if oldval == []:
 	    if DEBUG_MODE:
 	        syslog('info','oldval is an empty list.\nThis can happen either because of\n 1)Permission issues (Do a: bin/check_perms)\n 2)Inconsistency between database and pickle files (A user is in the database but not in pickle files or vice versa,Do a bin/find_problems.py)')
         oldval = oldval[0]
 	if DEBUG_MODE:
-            syslog('info','the value of oldval is %s:',oldval)
+            syslog('info','the value of oldval is %s:', oldval)
 
         if flag:
             newval = oldval | 1          # Disable delivery
         else:
             newval = oldval & ~1          # Enable delivery
         if DEBUG_MODE:
-            syslog('info','the value of newval is %s:',newval)
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(suppress = newval)\n"
+            syslog('info','the value of newval is %s:', newval)
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(suppress = newval)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDisable):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(suppress = newval)       
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(suppress = newval)       
 
     @decfunc
     def setDigest(self, member, flag):
         """Disable/enable delivery based on user digest status"""
 
-        command = "result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
+        command = "result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8'))\noldval = [(subscriber.suppress) for subscriber in result]\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDigest):Executing query:\n%s', command)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8"))
         oldval = [(subscriber.suppress) for subscriber in result]
 	if oldval == []:
 	    if DEBUG_MODE:
 	        syslog('info','oldval is an empty list.\nThis can happen either because of\n 1)permission issues (Do a: bin/check_perms)\n 2)Inconsistency between database and pickle files (A user is in the database but not in pickle files or vice versa,Do a bin/find_problems.py)')
         oldval = oldval[0]
         if DEBUG_MODE:
-            syslog('info','value of oldval %s:',oldval)
+            syslog('info','value of oldval %s:', oldval)
 
         if flag:
             newval = oldval | 2          # Suppress delivery (in favor of digests)
         else:
             newval = oldval & ~2          # Enable delivery (instead of digests)
 
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(suppress = newval)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(suppress = newval)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(setDigest):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(suppress = newval)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(suppress = newval)
 
     @decfunc
     def changePreference(self, member, preference):
         """Change a user's default preference for new threads."""
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(preference = preference)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(preference = preference)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changePreference):Executing query:\n%s', command)    
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(preference = preference)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(preference = preference)
 
     @decfunc
     def changeFormat(self, member, format):
         """Change a user's preferred delivery format (plain text and/or html)"""
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,'utf-8')).set(format = format)\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,'utf-8')).set(format = format)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changeFormat):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(member,"utf-8")).set(format = format)    
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(member,"utf-8")).set(format = format)    
 
     ### We have to watch out for a tricky case (that actually happened):
     ### User [e-mail] changes her address to something already in the
@@ -207,32 +207,32 @@
     @decfunc
     def changeAddress(self, oldaddr, newaddr):
         """Change email address in SQL database"""
-        #if DEBUG_MODE:
-        #syslog('info', "Changing email address on %s from '%s' to '%s'", self.mlist.internal_name(), oldaddr, newaddr)
+        if DEBUG_MODE:
+        	syslog('info', "Changing email address on %s from '%s' to '%s'", self.mlist.internal_name(), oldaddr, newaddr)
         ## Check if newaddr is in sql database        
-        num_matches = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8")).count()
+        num_matches = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8")).count()
 
         if num_matches > 1:
-            #syslog('error', 'Multiple users with same key (%s): %s', self.mlist.internal_name(), newaddr)
+            syslog('error', 'Multiple users with same key (%s): %s', self.mlist.internal_name(), newaddr)
             return
         if num_matches == 1:
             self.mergeSubscribers(oldaddr, newaddr)
 
-        command = "num_matches = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,'utf-8')).count()\nself.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,'utf-8')).set(mailman_key = unicode(newaddr,'utf-8'))\nself.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,'utf-8')).set(deleted = False)\n"
+        command = "num_matches = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr,'utf-8')).count()\nself.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,'utf-8')).set(mailman_key = unicode(newaddr,'utf-8'))\nself.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),'utf-8')).set(deleted = False)\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(changeAddress):Executing query:\n%s', command)
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8")) 
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,"utf-8")).set(mailman_key = unicode(newaddr,"utf-8")) 
         self.store.commit()            
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8")).set(deleted = False)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8")).set(deleted = False)
 
     @decfunc
     def unsubscribeFromList(self, key):
         """Indicate that a user has unsubscribed by setting the deleted flag in the subscriber table."""
 
-        command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8')).set(deleted = True)\nself.store.find(Alias,Alias.subscriber_id == subscriber_id).remove()\n"
+        command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key,'utf-8')).set(deleted = True)\nself.store.find(Alias,Alias.subscriber_id == subscriber_id).remove()\n"
         if DEBUG_MODE:
             syslog('info', 'DlisUtils(unsubscribeFromList):Executing query:\n%s', command)          
-        self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8")).set(deleted = True)
+        self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key,"utf-8")).set(deleted = True)
         # get the subscriber id from the mailman_key & delete all aliases  
         subscriber_id = self.getSubscriber_id_raw(key)
 
@@ -245,20 +245,27 @@
     def subscribeToList(self, key):
         """Add a member to the subscriber database, or change the record from deleted if it was already present."""
 
-        command = "count = (self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8'))).count()\n"
+        command = "count = (self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),'utf-8'))).count()\n"
         if DEBUG_MODE:
             syslog('info', 'DlistUtils(subscribeToList):Executing query:\n%s', command)
         # First see if member is subscribed with deleted field = false
-        count = (self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8"))).count()
+        count = (self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8"))).count()
 
         if count:
-            command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8')).set(deleted = False)"
+            command = "self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),'utf-8')).set(deleted = False)"
             if DEBUG_MODE:
                 syslog('info', 'DlistUtils(subscribeToList):Executing query:\n%s', command)
-            self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,"utf-8")).set(deleted = False)
+            self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8")).set(deleted = False)
+
+	    result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8"))
+            Email = [(subscriber.mailman_key) for subscriber in result]
+	    Email = Email[0]
+	    if Email != key: #That is if one was in lowercase and other in uppercase then update mailman_key with case sensitivity same as key
+		self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(key.lower(),"utf-8")).set(mailman_key = unicode(key,"utf-8"))
+	    
         else:
             # format is text-only (1) by default
-            command = "subscriber = self.store.add(Subscriber())\nsubscriber.mailman_key = unicode(key,'utf-8')\nsubscriber.preference = 1\nsubscriber.deleted = False\nsubscriber.format = 1\nsubscriber.suppress = 0"
+            command = "subscriber = self.store.add(Subscriber())\nmailman_key = unicode(key,'utf-8')\npreference = 1\ndeleted = False\nformat = 1\nsuppress = 0"
             if DEBUG_MODE:
                 syslog('info', 'DlistUtils(subscribeToList)Executing query:\n%s', command)
             self.mailman_key = unicode(key,"utf-8")
@@ -273,12 +280,12 @@
         # use the original subscriber id as the ID going forward
         if DEBUG_MODE:
             syslog('info', 'Executing commands of mergeSubscribers:')
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(oldaddr,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(oldaddr,"utf-8"))
         new_id = [(subscriber.subscriber_id) for subscriber in result]
         new_id = new_id[0]
         if DEBUG_MODE:
             syslog('info', 'the value of new_id: %s', new_id)
-        result = self.store.find(Subscriber,Subscriber.mailman_key == unicode(newaddr,"utf-8"))
+        result = self.store.find(Subscriber,Subscriber.mailman_key.lower() == unicode(newaddr.lower(),"utf-8"))
         obsolete_id = [(subscriber.subscriber_id) for subscriber in result]
         obsolete_id = obsolete_id[0]    
         if DEBUG_MODE:
@@ -291,13 +298,13 @@
 
 class Message(object):
     __storm_table__ = "message"
-    message_id = Int(primary = True,default = AutoReload)
+    message_id = Int(primary = True, default = AutoReload)
     sender_id = Int()
-    subscriber = Reference(sender_id,Subscriber.subscriber_id)
+    subscriber = Reference(sender_id, Subscriber.subscriber_id)
     subject = Unicode()
     thread_id = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -314,7 +321,7 @@
             threadID = -1
         command = "message = self.store.add(Message())\nmessage.sender_id = senderID\nmessage.thread_id = threadID\nmessage.subject = unicode(subject,'utf-8')\nmessageID = self.store.find(Message).max(Message.message_id)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createMessage)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createMessage)executing query:\n%s', command)
         #message_id has autoreload set,its value will be serially updated in database 
         self.sender_id = senderID
         self.thread_id = threadID
@@ -322,19 +329,19 @@
         self.store.add(self) 
         messageID = self.store.find(Message).max(Message.message_id)     
         if DEBUG_MODE:
-            syslog('info','Result of query(messageID) is: %s\n',messageID)
+            syslog('info','Result of query(messageID) is: %s\n', messageID)
         return messageID
 
 class Thread(object):
     __storm_table__ = "thread"
-    thread_id = Int(primary = True,default = AutoReload)
+    thread_id = Int(primary = True, default = AutoReload)
     thread_name = Unicode()
     base_message_id = Int()
-    message = Reference(base_message_id,Message.message_id)
+    message = Reference(base_message_id, Message.message_id)
     status = Int(default = 0)
     parent = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -347,7 +354,7 @@
         senderID = subscriber.getSubscriber_id(msg, msgdata, safe=1, loose=1)
         command = "thread = self.store.add(Thread())\nthread.base_message_id = msgdata['message_id']\nthreadID = self.store.find(Thread).max(Thread.thread_id)\nself.store.find(Message,Message.message_id == msgdata['message_id']).set(thread_id = threadID)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createThread)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createThread)executing query:\n%s', command)
         self.base_message_id = msgdata['message_id']
         self.store.add(self)
         threadID = self.store.find(Thread).max(Thread.thread_id)          
@@ -370,7 +377,7 @@
         threadBase = threadBase[:13]
         command = "num = self.store.find(Thread,Thread.thread_name == unicode(threadBase,'utf-8')).count()\nself.store.find(Thread,Thread.thread_id == threadID).set(thread_name = threadName)\n"
         if DEBUG_MODE:
-            syslog('info','DlistUtils:(createThread)executing query:\n%s',command)
+            syslog('info','DlistUtils:(createThread)executing query:\n%s', command)
         # If the threadBase is not unique, make threadBase unique by appending a number
         num = self.store.find(Thread,Thread.thread_name.like(unicode(threadBase + "%","utf-8"))).count()
         if not num:
@@ -405,26 +412,24 @@
         msg['To'] =  '%s+%s@%s' % (self.mlist.internal_name(),
                              	   name,
                              	   self.mlist.host_name)    
-        for i in (1,2):
+        for i in (1, 2):
             # different footers for different prefs, so we need to queue separately
             if(i==1):
                 #For condition where preference = True
                 pref=True
-           #     command = "lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]\n"
-            #    if DEBUG_MODE:
-             #       syslog('info', 'DlistUtils:(newThread)executing query:\n%sfor pref = true\n', command)
+                if DEBUG_MODE:
+                    syslog('info', 'DlistUtils:(newThread)executing query:\nfor pref = true\n\n')
             if(i==2):
                 #For condition where preference = False
                 pref=False
-              #  command = "lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]\n"
-               # if DEBUG_MODE:
-                #    syslog('info', 'DlistUtils:(newThread)executing query:\n%sfor pref = false\n', command)
+                if DEBUG_MODE:
+                    syslog('info', 'DlistUtils:(newThread)executing query:\nfor pref = false\n\n')
 
             #Execute a SELECT statement, to find the list of matching subscribers.
             result_new_sql = self.store.find(Subscriber,And(Subscriber.preference == pref,Subscriber.deleted == False,Subscriber.suppress == 0))
             lists = [(subscriber.mailman_key.encode('utf-8')) for subscriber in result_new_sql]
             if DEBUG_MODE:
-                syslog('info', 'value of lists: %s\n', lists)
+                syslog('info','value of lists: %s\n', lists)
             self.email_recipients(msg, msgdata, lists, pref)
 
         # Make original message go to nobody (but be archived)
@@ -514,12 +519,12 @@
         override = Override(self.mlist)
         override.override(msg, msgdata, 0)
 
-    def alphanumericOnly(self,s):
+    def alphanumericOnly(self, s):
         """Filter any non-letter characters from a string"""
         result = [letter for letter in s if letter in string.ascii_letters or letter in string.digits]
         return string.join(result, '')
 
-    def subjectToName(self,subject, threadID):
+    def subjectToName(self, subject, threadID):
         """Return a lower-case name for a new thread based on the subject, if present, or on the threadID"""
         result = None
 
@@ -589,9 +594,9 @@
     __storm_table__ = "alias"    
     pseudonym = Unicode(primary = True)
     subscriber_id = Int()
-    subscriber = Reference(subscriber_id,Subscriber.subscriber_id)
+    subscriber = Reference(subscriber_id, Subscriber.subscriber_id)
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -604,8 +609,8 @@
              syslog('info', 'DlistUtils(canonicalize_sender)Executing query:\n%s', command)
 
         for alias in aliases:
-            #if DEBUG_MODE:
-                #syslog('info', 'Checking if <%s> is an alias', alias)
+            if DEBUG_MODE:
+                syslog('info', 'Checking if <%s> is an alias', alias)
             result = self.store.find((Subscriber,Alias),And(Alias.pseudonym == unicode(alias,'utf-8'),Subscriber.subscriber_id == Alias.subscriber_id))
             lists = [(subscriber.mailman_key.encode('utf-8')) for (subscriber,alias) in result]
 
@@ -654,12 +659,12 @@
     __storm_table__ = "override"
     __storm_primary__ = "subscriber_id", "thread_id"
     subscriber_id = Int()
-    subscriber = Reference(subscriber_id,Subscriber.subscriber_id)
+    subscriber = Reference(subscriber_id, Subscriber.subscriber_id)
     thread_id = Int()
-    thread = Reference(thread_id,Thread.thread_id)
+    thread = Reference(thread_id, Thread.thread_id)
     preference = Int()
 
-    def __init__(self,mlist):
+    def __init__(self, mlist):
         self.mlist = mlist
         self.database = getConn(mlist)
 
@@ -709,10 +714,9 @@
 
         return 1
 
-    def overrideURL(self,list_name, host_name, thread_reference, preference):
+    def overrideURL(self, list_name, host_name, thread_reference, preference):
         return 'http://%s/mailman/options/%s?override=%s&preference=%d' % (host_name, list_name, thread_reference, preference)
 
-
 #Functions Independent of the database tables in DlistUtils
 
 def GetEmailAddresses(mlist, subscribers):
@@ -752,13 +756,22 @@
 
 def _create_database(name):
     """Create a database. This requires us to not be in a transaction."""
-    conn = pgdb.connect(host='localhost', database='postgres',user='mailman', password='mailman')
+    conn = pgdb.connect(host='localhost', database='postgres', user='mailman', password='mailman')
     old_iso_lvl = conn.isolation_level
     conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
     cursor = conn.cursor()
     cursor.execute('create database "%s"' % name) 
     conn.set_isolation_level(old_iso_lvl)
 
+def _remove_database(name):
+    """Remove a database. This requires us to not be in a transaction."""
+    conn = pgdb.connect(host='localhost', database='postgres', user='mailman', password='mailman')
+    old_iso_lvl = conn.isolation_level
+    conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
+    cursor = conn.cursor()
+    cursor.execute('DROP DATABASE "%s"' % name) 
+    conn.set_isolation_level(old_iso_lvl)
+
 def executeSQL(mlist, commands):
     """Execute a sequence of SQL commands that create tables in the database."""
     database = getConn(mlist)
@@ -834,3 +847,9 @@
     mlist.Save()
     if alreadyLocked == 0:
         mlist.Unlock()
+
+def remove_dlist(listname):
+    """ Deletes the corresponding postgres database and tables."""
+    _remove_database(listname)
+    if DEBUG_MODE:
+         syslog('info', "Database %s removed\n", listname)

=== modified file 'Mailman/mm_cfg.py'
--- Mailman/mm_cfg.py	2010-01-31 20:29:48 +0000
+++ Mailman/mm_cfg.py	2010-05-28 04:55:39 +0000
@@ -68,4 +68,4 @@
 STORM_DB_HOST = "localhost"
 
 # Variable to write more to the logs when in debugging mode (currently only used for the info-logs). Set to False in production mode.
-DEBUG_MODE = True 
+DEBUG_MODE = False 

=== modified file 'bin/rmlist'
--- bin/rmlist	2009-07-31 10:49:01 +0000
+++ bin/rmlist	2010-05-28 04:55:39 +0000
@@ -152,12 +152,7 @@
 
     # Remove the database in PostgreSQL if it was a dynamic list. Added by Systers.
     try:
-        conn = DlistUtils.getConn(None)
-        old_iso_lvl = conn.isolation_level
-        conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
-        cursor = conn.cursor()
-        cursor.execute('DROP DATABASE "%s"' % listname)
-        conn.set_isolation_level(old_iso_lvl)
+        DlistUtils.remove_dlist(listname)
     except:
         syslog('error', 'Tried to remove the database for %s, no such database (ok if it was a non dlist). If it was a dlist, the postgreSQL database for this list needs to be removed manually.' % listname)