[Systers-dev] Latest round of bugs on systers.org
P. A. Grubel
pagrubel at tularosa.net
Mon Jan 25 06:39:45 PST 2010
I would love to help test and even look at the code. Please direct me
Thanks,
Pat
-----Original Message-----
We've found some bugs in systers that are causing grief, and we need to do a
test and release cycle to deal with them. We'd love to have some of you
help out (a few of you have been tapped on the shoulder already. You know
who you are). The more people we can get working on this, the faster we can
release the new code (and not have these conversations halt in mid
conversation, because all the replies are being lost). If we can get 5-6
testers, it should only take 1-2 hours each, and it's a way to get started
being familiar with mailman and the tools we use, so that you can do
something perhaps more up your alley next time.
Kathy Gee will be the testing coordinator this time, but we'd love to have
someone shadow her who could do it next time. Someone who has or wants to
develop project management skills would find this interesting. Kathy will
tell you what she did/does, so that you can learn the ropes of this job.
I want to use this message to explain the bugs we encountered and how we
fixed them.
1. The simplist bug was one created by a malformed URL. Not sure what
triggered this, but I suspect someone miscopied a link from their email to
their browser and lost the last couple of characters. When you try to
unsubscribe from a conversation, a url is created with two GET parameters
(it looks like
www.systers.org/mailman/options/systers?override=47&preference=0 )
override is the number of the conversation being "overridden" (being
unsubscribed to) and preference is what you want to do -- 0 is unsubscribe,
1 is subscribe (once you have unsubscribed).
If you lose the last character or so of the url, then preference does not
have a value. The code actually checks for this (this is in
Mailman/Cgi/options.py, for those of you who have the code available to you)
and sets them to the empty string (all URL parameters are initially treated
as strings). However, there are two things wrong
- it later doesn't use these variables that have been set to the value of
those parameters (or the defaults)
- even when it does, it tries to convert preference to a number (using
int() ), and that doesn't work on the empty string.
The change was to use the computed variables in all the places the raw
values were being recomputed, and to change the default of preference to '0'
(the string zero), so that if it didn't have a value, it would try to
unsubscribe you. If that assumption is wrong, you are presumably already
unsubscribed, so either you would notice before you had to submit the page,
since all the text (including the button label) would be about
unsubscribing, or you would just stay unsubscribed.
The other two bugs had to do with our change from raw data base calls to a
database abstraction layer (we added this in November, and in the long run
it will give us a lot of flexibility). For those who know about these
things, we are using STORM.
2. When there is a reply to a message it needs to go to all the people:
- who are not deleted
- who don't get the digest
- who have a default of getting replies and have NOT changed their
preference for this thread
- who have a default to not get replies and HAVE changed their preference
for this thread
That was a pretty convoluted SQL statement, and the code does some
optimizing behind the scenes, where it took a clause (each of the last two
cases) with a subtle error in it and sent it do the database as true or
false (that is, it believed the result of that computation would always be
true/false, so why bother to make the database do the computation). We
learned from this that database logging is your friend, as once you see it
in the database logs, it's pretty obvious it's a bug. The result was that
replies to threads where at least one person had changed their preference
went only to the digest, stopping conversation on a number of interesting
threads.
We solved it by refactoring this code, breaking it into smaller pieces that
make the error more obvious (learning to write readable code is a practice
we all can get better at).
3. New threads on systers-mailman are started by sending mail to
systers+new at systers.org <systers%2Bnew at systers.org> and responded by by
sending to systers+threadName at systers.org
<systers%2BthreadName at systers.org> Mailman has to choose a thread
name. To make it sensible and not a random
string, it looks for the first long word in the subject line (long means 6
characters or more). That can lead to duplicate thread names, so it adds a
number to the end for the second and later items. It finds what number to
use by asking the database how many threads have the name threadName. The
problem is that the database always returns 0 or 1, because the variants
other than the first have the name threadName2 or threadName47. So whenever
a new thread started that needed to reuse a thread name, it became
threadName2 and the preferences of users for whether they got messages for
threadName2 were reused (and I believe this interacted with bug 2, in that,
if a thread had existed previously, the odds were high that someone had
unsubscribed to it, so no one would get the replies). I suspect this bug
has been around forever, but until bug 2 happened, we didn't notice it,
because it rarely impacted people.
This was fixed by changing from an SQL test for equality to the use of LIKE,
which is like a regular expression test.
The changes to these are in Mailman/DlistUtils.py, for those who want to
check this out.
Love to get questions from people who want to understand this better; I can
point you to the code, where you can see how it fits into the larger
context.
Robin
To unsubscribe from this conversation, send email to
<systers-dev+systersorg+unsubscribe at systers.org> or visit
<http://systers.org/mailman/options/systers-dev?override=58&preference=0>
To contribute to this conversation, use your mailer's reply-all or
reply-group command or send your message to
systers-dev+systersorg at systers.org
To start a new conversation, send email to <systers-dev+new at systers.org>
To unsubscribe entirely from systers-dev, send email to
<systers-dev-request at systers.org> with subject unsubscribe.
No virus found in this incoming message.
Checked by AVG - www.avg.com
Version: 9.0.730 / Virus Database: 271.1.1/2642 - Release Date: 01/24/10
00:33:00
To contribute to this conversation, send mail to <>
More information about the Systers-dev
mailing list