GNU Mailman

Merge lp:~danci-emanuel/mailman/messages_support into lp:~danci-emanuel/mailman/dlist_runner

  1. messages_support
  2. Merge into dlist_runner
Proposed by Emanuel Danci
Status: Needs review
Proposed branch: lp:~danci-emanuel/mailman/messages_support
Merge into: lp:~danci-emanuel/mailman/dlist_runner
Diff against target: 147 lines (+25/-6)
8 files modified
src/mailman/app/moderator.py (+1/-1)
src/mailman/database/schema/postgres.sql (+1/-0)
src/mailman/database/schema/sqlite.sql (+1/-0)
src/mailman/interfaces/mailinglist.py (+1/-1)
src/mailman/interfaces/messages.py (+5/-1)
src/mailman/model/message.py (+4/-1)
src/mailman/model/messagestore.py (+11/-2)
src/mailman/rules/max_size.py (+1/-0)
To merge this branch: bzr merge lp:~danci-emanuel/mailman/messages_support
Reviewer Review Type Date Requested Status
Terri Pending
Robin J Pending
Review via email: mp+124798@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Emanuel Danci (danci-emanuel) wrote :

Just some small comments:

1. This is only the part that will be used by the handlers when creating the new messags. (the equivalent of the Message class from DlistUtils)

2. I did not push the code for the other parts of the DlistUtils (the ones related to the Thread and Override classes) because I still have to finish some parts of them and I also took you advice, pushing smaller pieces of code, so that they will be easier to review.

3. I also changed the attribute from mailinglist.py, the one that you told me to, and added the missing line to max_size.py.

4. I tried to add Terri as I reviewer to, but I cannot seem to find her on launchpad.

Reply
Revision history for this message
Robin J (robin-jeffries) wrote :

> Just some small comments:
>
> 1. This is only the part that will be used by the handlers when creating the
> new messags. (the equivalent of the Message class from DlistUtils)

OK, I don't think I understand why existing mailman doesn't include the msgdata, but you need to. Can you explain it to me? (It's probably because I don't know the mm3 code, but it will help us both if you can explain it.) What is in the metadata?

>
> 2. I did not push the code for the other parts of the DlistUtils (the ones
> related to the Thread and Override classes) because I still have to finish
> some parts of them and I also took you advice, pushing smaller pieces of code,
> so that they will be easier to review.

Good.

>
> 3. I also changed the attribute from mailinglist.py, the one that you told me
> to, and added the missing line to max_size.py.

You mean the help text associated with that attribute?

>
> 4. I tried to add Terri as I reviewer to, but I cannot seem to find her on
> launchpad.

I can't figure out how to add another reviewer from this view, but Terri is terriko on launchpad.

Reply
Revision history for this message
Emanuel Danci (danci-emanuel) wrote :

> > Just some small comments:
> >
> > 1. This is only the part that will be used by the handlers when creating the
> > new messags. (the equivalent of the Message class from DlistUtils)
>
> OK, I don't think I understand why existing mailman doesn't include the
> msgdata, but you need to. Can you explain it to me? (It's probably because I
> don't know the mm3 code, but it will help us both if you can explain it.)
> What is in the metadata?
>

Right not, mm3 only stores the following information for a certain message: the message_id_hash, its path and the message_id, which are all found in the msg param. I added the msgdata because it contains the 'thread_id' field, which will be added by the one of the two handlers (either FirstDlist or SecondDlist, as Sneha Priscilla called them), when the message gets there.

> >
> > 2. I did not push the code for the other parts of the DlistUtils (the ones
> > related to the Thread and Override classes) because I still have to finish
> > some parts of them and I also took you advice, pushing smaller pieces of
> code,
> > so that they will be easier to review.
>
> Good.
>
> >
> > 3. I also changed the attribute from mailinglist.py, the one that you told
> me
> > to, and added the missing line to max_size.py.
>
> You mean the help text associated with that attribute?

Yes, the text.

> >
> > 4. I tried to add Terri as I reviewer to, but I cannot seem to find her on
> > launchpad.
>
> I can't figure out how to add another reviewer from this view, but Terri is
> terriko on launchpad.

Thank you for Terri`s id! I added her to the reviewers list, too!

Reply
Revision history for this message
Robin J (robin-jeffries) wrote :

> > > Just some small comments:
> > >
> > > 1. This is only the part that will be used by the handlers when creating
> the
> > > new messags. (the equivalent of the Message class from DlistUtils)
> >
> > OK, I don't think I understand why existing mailman doesn't include the
> > msgdata, but you need to. Can you explain it to me? (It's probably because
> I
> > don't know the mm3 code, but it will help us both if you can explain it.)
> > What is in the metadata?
> >
>
> Right not, mm3 only stores the following information for a certain message:
> the message_id_hash, its path and the message_id, which are all found in the
> msg param. I added the msgdata because it contains the 'thread_id' field,
> which will be added by the one of the two handlers (either FirstDlist or
> SecondDlist, as Sneha Priscilla called them), when the message gets there.
>

So, msg is an object, so why not just make the thread_id an attribute of the object? (I don't want us to get to far away from what mailman is doing, and that seems to be what mailman is doing with the other info about the message)

I'm refering to
  moderator.py
  messages.py

Reply
Revision history for this message
Emanuel Danci (danci-emanuel) wrote :

> > > > Just some small comments:
> > > >
> > > > 1. This is only the part that will be used by the handlers when creating
> > the
> > > > new messags. (the equivalent of the Message class from DlistUtils)
> > >
> > > OK, I don't think I understand why existing mailman doesn't include the
> > > msgdata, but you need to. Can you explain it to me? (It's probably
> because
> > I
> > > don't know the mm3 code, but it will help us both if you can explain it.)
> > > What is in the metadata?
> > >
> >
> > Right not, mm3 only stores the following information for a certain message:
> > the message_id_hash, its path and the message_id, which are all found in the
> > msg param. I added the msgdata because it contains the 'thread_id' field,
> > which will be added by the one of the two handlers (either FirstDlist or
> > SecondDlist, as Sneha Priscilla called them), when the message gets there.
> >
>
> So, msg is an object, so why not just make the thread_id an attribute of the
> object? (I don't want us to get to far away from what mailman is doing, and
> that seems to be what mailman is doing with the other info about the message)
>
> I'm refering to
> moderator.py
> messages.py

Yes, theoretically that can be done. So than I should go with changing the initial dlist model and add the 'thread_id' to msg instead of adding it to the msgdata? This means that I will only have to keep in mind to search the thread_id in the msg object instead of the msgdata one, when doing the rest of the actions from DlistUtils.

Reply
Revision history for this message
Robin J (robin-jeffries) wrote :

> > So, msg is an object, so why not just make the thread_id an attribute of the
> > object? (I don't want us to get to far away from what mailman is doing, and
> > that seems to be what mailman is doing with the other info about the
> message)
> >
> > I'm refering to
> > moderator.py
> > messages.py
>
> Yes, theoretically that can be done. So than I should go with changing the
> initial dlist model and add the 'thread_id' to msg instead of adding it to the
> msgdata? This means that I will only have to keep in mind to search the
> thread_id in the msg object instead of the msgdata one, when doing the rest of
> the actions from DlistUtils.

Yes, this makes it easier for you, and doesn't confuse a future reader of the code about why thread_id is different from other data related to the message. (The old dlists were a private addon, so they wanted to keep as much separate from existing mm objects as possible, but this work is going into core mailman 3, so it's more important that it be parallel to other parts of that code)

Reply

Unmerged revisions

7165. By Emanuel Danci

Added the missing part from the messages.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mailman/app/moderator.py'
2--- src/mailman/app/moderator.py 2012-03-15 22:48:41 +0000
3+++ src/mailman/app/moderator.py 2012-09-17 21:12:24 +0000
4@@ -90,7 +90,7 @@
5 msg['Message-ID'] = message_id = unicode(make_msgid())
6 assert isinstance(message_id, unicode), (
7 'Message-ID is not a unicode: %s' % message_id)
8- getUtility(IMessageStore).add(msg)
9+ getUtility(IMessageStore).add(msg, msgdata)
10 # Prepare the message metadata with some extra information needed only by
11 # the moderation interface.
12 msgdata['_mod_message_id'] = message_id
13
14=== modified file 'src/mailman/database/schema/postgres.sql'
15--- src/mailman/database/schema/postgres.sql 2012-09-11 18:17:03 +0000
16+++ src/mailman/database/schema/postgres.sql 2012-09-17 21:12:24 +0000
17@@ -291,6 +291,7 @@
18 message_id_hash BYTEA,
19 path BYTEA,
20 message_id TEXT,
21+ thread_id_dlist INTEGER,
22 PRIMARY KEY (id)
23 );
24
25
26=== modified file 'src/mailman/database/schema/sqlite.sql'
27--- src/mailman/database/schema/sqlite.sql 2012-09-11 18:17:03 +0000
28+++ src/mailman/database/schema/sqlite.sql 2012-09-17 21:12:24 +0000
29@@ -239,6 +239,7 @@
30 message_id_hash TEXT,
31 path TEXT,
32 message_id TEXT,
33+ thread_id_dlist INTEGER,
34 PRIMARY KEY (id)
35 );
36
37
38=== modified file 'src/mailman/interfaces/mailinglist.py'
39--- src/mailman/interfaces/mailinglist.py 2012-09-11 18:17:03 +0000
40+++ src/mailman/interfaces/mailinglist.py 2012-09-17 21:12:24 +0000
41@@ -133,7 +133,7 @@
42 available mailing lists.""")
43
44 dlists_enabled = Attribute(
45- """Is the Dynamic sublists feature enabled for this list?""")
46+ """Allow people to (un)subscribe to individual conversations?""")
47
48 # Contact addresses
49
50
51=== modified file 'src/mailman/interfaces/messages.py'
52--- src/mailman/interfaces/messages.py 2012-01-01 19:14:46 +0000
53+++ src/mailman/interfaces/messages.py 2012-09-17 21:12:24 +0000
54@@ -62,12 +62,13 @@
55 http://archive.example.com/RXTJ357KFOTJP3NFJA6KMO65X7VQOHJI
56 """
57
58- def add(message):
59+ def add(message, msgdata):
60 """Add the message to the store.
61
62 :param message: An email.message.Message instance containing at least
63 a unique Message-ID header. The message will be given an
64 X-Message-ID-Hash header, overriding any existing such header.
65+ :param msgdata: The message metadata.
66 :returns: The calculated X-Message-ID-Hash header.
67 :raises ValueError: if the message is missing a Message-ID header.
68 The storage service is also allowed to raise this exception if it
69@@ -109,3 +110,6 @@
70 message_id_hash = Attribute("""The unique SHA1 hash of the message.""")
71
72 path = Attribute("""The filesystem path to the message object.""")
73+
74+ thread_id_dlist = Attribute("""The id of the thread that the message
75+ belongs to.""")
76
77=== modified file 'src/mailman/model/message.py'
78--- src/mailman/model/message.py 2012-04-26 02:08:22 +0000
79+++ src/mailman/model/message.py 2012-09-17 21:12:24 +0000
80@@ -41,12 +41,15 @@
81 message_id = Unicode()
82 message_id_hash = RawStr()
83 path = RawStr()
84+ thread_id_dlist = Int()
85 # This is a Messge-ID field representation, not a database row id.
86
87 @dbconnection
88- def __init__(self, store, message_id, message_id_hash, path):
89+ def __init__(self, store, message_id, message_id_hash, path,
90+ thread_id_dlist):
91 super(Message, self).__init__()
92 self.message_id = message_id
93 self.message_id_hash = message_id_hash
94 self.path = path
95+ self.thread_id_dlist = thread_id_dlist
96 store.add(self)
97
98=== modified file 'src/mailman/model/messagestore.py'
99--- src/mailman/model/messagestore.py 2012-04-26 02:08:22 +0000
100+++ src/mailman/model/messagestore.py 2012-09-17 21:12:24 +0000
101@@ -51,9 +51,12 @@
102 """See `IMessageStore`."""
103
104 @dbconnection
105- def add(self, store, message):
106+ def add(self, store, message, msgdata):
107+ return
108 # Ensure that the message has the requisite headers.
109 message_ids = message.get_all('message-id', [])
110+ for i in range(len(message_ids)):
111+ print( message_ids[i] )
112 if len(message_ids) <> 1:
113 raise ValueError('Exactly one Message-ID header required')
114 # Calculate and insert the X-Message-ID-Hash.
115@@ -76,13 +79,19 @@
116 parts.append(split.pop(0) + split.pop(0))
117 parts.append(hash32)
118 relpath = os.path.join(*parts)
119+ # extract the thread_id
120+ try:
121+ thread_id = msgdata['thread_id']
122+ except:
123+ thread_id = -1
124 # Store the message in the database. This relies on the database
125 # providing a unique serial number, but to get this information, we
126 # have to use a straight insert instead of relying on Elixir to create
127 # the object.
128 row = Message(message_id=message_id,
129 message_id_hash=hash32,
130- path=relpath)
131+ path=relpath,
132+ thread_id_dlist=thread_id)
133 # Now calculate the full file system path.
134 path = os.path.join(config.MESSAGES_DIR, relpath)
135 # Write the file to the path, but catch the appropriate exception in
136
137=== modified file 'src/mailman/rules/max_size.py'
138--- src/mailman/rules/max_size.py 2012-09-11 17:20:51 +0000
139+++ src/mailman/rules/max_size.py 2012-09-17 21:12:24 +0000
140@@ -3,6 +3,7 @@
141 # This file is part of GNU Mailman.
142 #
143 # GNU Mailman is free software: you can redistribute it and/or modify it under
144+# the terms of the GNU General Public License as published by the Free
145 # Software Foundation, either version 3 of the License, or (at your option)
146 # any later version.
147 #

Subscribers

People subscribed via source and target branches

to all changes: