launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02588
[Merge] lp:~lifeless/launchpad/bug-704446 into lp:launchpad
Robert Collins has proposed merging lp:~lifeless/launchpad/bug-704446 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-704446/+merge/49580
This wraps up the transition to having BugMessage.index always populated by deleting the migration only code.
--
https://code.launchpad.net/~lifeless/launchpad/bug-704446/+merge/49580
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-704446 into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-02-10 23:16:48 +0000
+++ lib/lp/bugs/configure.zcml 2011-02-14 00:26:57 +0000
@@ -732,7 +732,6 @@
messages
getBugNotificationRecipients
hasBranch
- reindexMessages
security_related
tags
getMessageChunks
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-02-10 14:20:12 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-02-14 00:26:57 +0000
@@ -466,9 +466,6 @@
def unsubscribeFromDupes(person, unsubscribed_by):
"""Remove this person's subscription from all dupes of this bug."""
- def reindexMessages():
- """Fill in the `BugMessage`.index column for this bugs messages."""
-
def isSubscribed(person):
"""Is person subscribed to this bug?
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-02-11 04:22:45 +0000
+++ lib/lp/bugs/model/bug.py 2011-02-14 00:26:57 +0000
@@ -434,34 +434,12 @@
"""See `IBug`."""
return self.users_affected_with_dupes.count()
- def reindexMessages(self):
- """See `IBug`."""
- store = Store.of(self)
- if store is None:
- # Bug hasn't been flushed yet, so use the store policy instead.
- store = IStore(BugMessage)
- if (store.find(BugMessage, BugMessage.bugID==self.id,
- BugMessage.index == None).count() == 0):
- # Fully indexed
- return
- indexed_messages = self._indexed_messages(include_bugmessage=True)
- # First reset the ones that have changed, so that we don't run into
- # IntegrityError due to having two temporarily hold the same index.
- for indexed_message, bugmessage in indexed_messages:
- if bugmessage.index != indexed_message.index:
- bugmessage.index = None
- Store.of(self).flush()
- for indexed_message, bugmessage in indexed_messages:
- if bugmessage.index != indexed_message.index:
- bugmessage.index = indexed_message.index
-
@property
def indexed_messages(self):
"""See `IMessageTarget`."""
return self._indexed_messages(include_content=True)
- def _indexed_messages(self, include_content=False, include_parents=True,
- include_bugmessage=False):
+ def _indexed_messages(self, include_content=False, include_parents=True):
"""Get the bugs messages, indexed.
:param include_content: If True retrieve the content for the messages
@@ -469,17 +447,12 @@
:param include_parents: If True retrieve the object for parent
messages too. If False the parent attribute will be *forced* to
None to reduce database lookups.
- :param include_bugmessage: If True returns tuples (message,
- bugmessage). This exists for the indexMessages API.
"""
# Make all messages be 'in' the main bugtask.
inside = self.default_bugtask
store = Store.of(self)
message_by_id = {}
- if include_parents or include_bugmessage:
- to_messages = lambda rows: [row[0] for row in rows]
- else:
- to_messages = lambda rows: rows
+ to_messages = lambda rows: [row[0] for row in rows]
def eager_load_owners(messages):
# Because we may have multiple owners, we spend less time
@@ -512,35 +485,29 @@
cache.text_contents = Message.chunks_text(
chunk_map[message.id])
- def eager_load(rows, slice_info):
+ def eager_load(rows):
messages = to_messages(rows)
eager_load_owners(messages)
if include_content:
eager_load_content(messages)
- def index_message(row, index):
+ def index_message(row):
# convert row to an IndexedMessage
if include_parents:
- if include_bugmessage:
- message, parent, bugmessage = row
- else:
- message, parent = row
+ message, parent, bugmessage = row
if parent is not None:
# If there is an IndexedMessage available as parent, use
# that to reduce on-demand parent lookups.
parent = message_by_id.get(parent.id, parent)
else:
- if include_bugmessage:
- message, bugmessage = row
- else:
- message = row
+ message, bugmessage = row
parent = None # parent attribute is not going to be accessed.
+ index = bugmessage.index
result = IndexedMessage(message, inside, index, parent)
- # This message may be the parent for another: stash it to permit
- # use.
- message_by_id[message.id] = result
- if include_bugmessage:
- result = result, bugmessage
+ if include_parents:
+ # This message may be the parent for another: stash it to
+ # permit use.
+ message_by_id[message.id] = result
return result
# There is possibly some nicer way to do this in storm, but
# this is a lot easier to figure out.
@@ -553,25 +520,21 @@
parent_message.id in (
select bugmessage.message from bugmessage where bugmessage.bug=%s)),
BugMessage""" % sqlvalues(self.id))
- lookup = Message, ParentMessage
- if include_bugmessage:
- lookup = lookup + (BugMessage,)
+ lookup = Message, ParentMessage, BugMessage
results = store.using(tables).find(
lookup,
BugMessage.bugID == self.id,
BugMessage.messageID == Message.id,
)
else:
- lookup = Message
- if include_bugmessage:
- lookup = lookup, BugMessage
+ lookup = Message, BugMessage
results = store.find(lookup,
BugMessage.bugID == self.id,
BugMessage.messageID == Message.id,
)
- results.order_by(Message.datecreated, Message.id)
+ results.order_by(BugMessage.index, Message.datecreated, Message.id)
return DecoratedResultSet(results, index_message,
- pre_iter_hook=eager_load, slice_info=True)
+ pre_iter_hook=eager_load)
@property
def displayname(self):
@@ -1127,7 +1090,6 @@
if message not in self.messages:
if user is None:
user = message.owner
- self.reindexMessages()
result = BugMessage(bug=self, message=message,
bugwatch=bugwatch, remote_comment_id=remote_comment_id,
index=self.bug_messages.count())
=== modified file 'lib/lp/bugs/model/bugmessage.py'
--- lib/lp/bugs/model/bugmessage.py 2011-02-10 01:18:39 +0000
+++ lib/lp/bugs/model/bugmessage.py 2011-02-14 00:26:57 +0000
@@ -45,9 +45,8 @@
notNull=False, default=None)
remote_comment_id = StringCol(notNull=False, default=None)
visible = BoolCol(notNull=True, default=True)
- # -- The index of the message is cached in the DB. Currently optional until
- # we migrate all data. Bug 704446 has info about the migration.
- index = IntCol(default=None)
+ # -- The index of the message is cached in the DB.
+ index = IntCol(default=None, notNull=True)
def __repr__(self):
return "<BugMessage at 0x%x message=%s index=%s>" % (
@@ -65,7 +64,6 @@
parent=bug.initial_message, owner=owner,
rfc822msgid=make_msgid('malone'), subject=subject)
chunk = MessageChunk(message=msg, content=content, sequence=1)
- bug.reindexMessages()
bugmsg = BugMessage(bug=bug, message=msg,
index=bug.bug_messages.count())
=== modified file 'lib/lp/bugs/tests/test_bug_messages.py'
--- lib/lp/bugs/tests/test_bug_messages.py 2011-01-31 09:24:13 +0000
+++ lib/lp/bugs/tests/test_bug_messages.py 2011-02-14 00:26:57 +0000
@@ -35,8 +35,3 @@
# IIndexedMessage.
for indexed_message in self.bug_2.indexed_messages:
self.failUnlessEqual(None, indexed_message.parent)
-
- def test_reindexMessages_indexes_messages(self):
- self.bug_2.bug_messages[0].index = None
- self.bug_2.reindexMessages()
- self.assertEqual(0, self.bug_2.bug_messages[0].index)
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2011-02-04 19:31:22 +0000
+++ lib/lp/scripts/garbo.py 2011-02-14 00:26:57 +0000
@@ -53,7 +53,6 @@
from lp.bugs.interfaces.bug import IBugSet
from lp.bugs.model.bug import Bug
from lp.bugs.model.bugattachment import BugAttachment
-from lp.bugs.model.bugmessage import BugMessage
from lp.bugs.model.bugnotification import BugNotification
from lp.bugs.model.bugwatch import BugWatch
from lp.bugs.scripts.checkwatches.scheduler import (
@@ -531,31 +530,6 @@
% chunk_size)
-class BugMessageIndexer(TunableLoop):
- """Index `BugMessage` in the DB to allow smarter queries in future.
-
- We find bugs with unindexed messages, and ask the bug to index them.
- """
- maximum_chunk_size = 10000
-
- def _to_process(self):
- return IMasterStore(BugMessage).find(
- BugMessage.bugID,
- BugMessage.index==None).config(distinct=True)
-
- def isDone(self):
- return self._to_process().any() is None
-
- def __call__(self, chunk_size):
- chunk_size = int(chunk_size)
- bugs_to_index = list(self._to_process()[:chunk_size])
- bugs = IMasterStore(Bug).find(Bug, Bug.id.is_in(bugs_to_index))
- for bug in bugs:
- bug.reindexMessages()
- transaction.commit()
- self.log.debug("Indexed %d bugs" % len(bugs_to_index))
-
-
class BugNotificationPruner(TunableLoop):
"""Prune `BugNotificationRecipient` records no longer of interest.
@@ -897,7 +871,6 @@
RevisionCachePruner,
BugHeatUpdater,
BugWatchScheduler,
- BugMessageIndexer,
]
experimental_tunable_loops = []
