Merge lp:~robru/queuebot/fix-rtm-dupes into lp:queuebot

[Robert Bruce Park]

Stop flooding pings when spreadsheet column A isn't unique.

Apologies for the big diff, but I was able to track down the flooding problem. What was happening was that people were copy&pasting their utopic landings as RTM landings, so spreadsheet column A which we were indexing on was no longer unique. And since the utopic/RTM silos rarely had identical statuses, the bot would see one status, see the other status, see they were different, and ping that, always.

I solved that by creating a more unique token by hashing together columns A and H, which is to say I combine the destination release with the landing description to create a more unique key to index off of.

In the process of writing this, I realized that the plugin had a memory leak, because it never cleared old descriptions that it was scanning, so in theory those were just piling up forever. I solved it by creating a new dict each time rather than adding to the same old dict forever.

Additionally, I wrote a few simplifications that make the code easier to read, at least for me. Hopefully none of this is controversial for you.

Thanks!

[Robert Bruce Park (robru)]

I tested this in the staging channel and it looked really good, didn't flood at all. Please merge! ;-)

review: Approve

=== modified file 'plugins/landing.py'
--- plugins/landing.py	2014-07-16 18:32:07 +0000
+++ plugins/landing.py	2014-08-26 23:10:20 +0000
@@ -11,6 +11,9 @@
       "?key=0AuDk72Lpx8U5dFVHQ3FuMDJGLUZCamJfSjYzbWh3Wnc" \
       "&usp=sharing&output=csv"
 
+TESTED = "trainguards, {silo_name}: {new_status}"
+ASSIGN = "trainguards, please assign line {line_num} for {landers}"
+
 
 class COL:
     pass
@@ -23,13 +26,6 @@
         try:
             self.notices = list()
 
-            if self.queue not in self.testing_state:
-                self.testing_state[self.queue] = dict()
-
-            # In verbose mode, show the current content of the queue
-            if self.verbose and self.queue not in self.landing_state:
-                self.landing_state[self.queue] = set()
-
             # Fetch current state
             opener = urllib2.build_opener(
                 urllib2.HTTPCookieProcessor(cookielib.CookieJar()))
@@ -47,6 +43,7 @@
                 setattr(COL, re.split('\W', heading.lower())[0], i)
 
             landing_ready = set()
+            testing_ready = dict()
             for line_num, line in enumerate(reader, start=4):
                 # Do some basic validation
                 if len(line) < 17:
@@ -55,46 +52,41 @@
                 description = line[COL.description]
                 new_status = line[COL.computed]
                 silo_name = line[COL.assigned]
-                landers = line[COL.lander]
+                landers = ", ".join(line[COL.lander].split())
                 ready = line[COL.ready]
                 request = line[COL.request]
-
-                old_status = self.testing_state[self.queue].get(description)
+                target = line[COL.target]
+                token = hash(target + description)
+
+                old_status = self.testing_state.get(token)
+
+                testing_ready[token] = new_status
                 if old_status != new_status:
-                    self.testing_state[self.queue][description] = new_status
                     if "Testing pass" in new_status:
-                        self.notices.append((
-                            "%s: trainguards, silo %s: %s"
-                            % (self.queue, silo_name, new_status),
-                            ("landing",)))
+                        self.notices.append((TESTED.format(**locals()),
+                                            (self.queue,)))
 
                 if ready != "Yes" or request:
                     continue
 
-                landing_ready.add(description)
-
-                if self.queue not in self.landing_state:
-                    continue
-
-                if self.queue in self.landing_state and \
-                        description in self.landing_state[self.queue]:
-                    continue
-
-                self.notices.append(("%s: trainguards, new landing "
-                                     "ready for assignment "
-                                     "(line %s, lander is %s)"
-                                     % (self.queue, line_num,
-                                        ", ".join(landers.split())),
-                                     ("landing",)))
-
-            self.landing_state[self.queue] = landing_ready
+                landing_ready.add(token)
+
+                if token in self.landing_state:
+                    continue
+
+                self.notices.append((ASSIGN.format(**locals()), (self.queue,)))
+
+            self.landing_state.clear()
+            self.landing_state.update(landing_ready)
+            self.testing_state.clear()
+            self.testing_state.update(testing_ready)
         except:
             # We don't want the bot to crash when something fails
             traceback.print_exc()
 
 
 class Landing():
-    landing_state = dict()
+    landing_state = set()
     testing_state = dict()
     scanner = LandingScanner()
     name = "landing"