Changeset 174248 in webkit


Ignore:
Timestamp:
Oct 2, 2014 4:49:30 PM (10 years ago)
Author:
ap@apple.com
Message:

update-work-items should never delete items
​https://bugs.webkit.org/show_bug.cgi?id=137366

Reviewed by Ryosuke Niwa.

As we don't just replace the whole list any more, indicate which items are high
priority, and which are not. Hight priority ones will be prepended to the queue,
others will be appended.

This creates a bit of unfairness in that high priority item queue becomes a LIFO.
But hopefully we will never have many rollouts competing like that.

  • QueueStatusServer/app.yaml: Update version.
  • QueueStatusServer/handlers/updateworkitems.py: Never remove items. Pass high

priority and regular items separately. Removed some error handling that used to
end up in returning status 500 - an uncaught exception does that automatically.

  • QueueStatusServer/main.py: Removed unnecessary regexps from URL matching code.
  • QueueStatusServer/model/workitems.py: Added a way to add high priority items.
  • QueueStatusServer/templates/updateworkitems.html: Added a field for high

priority items.

  • Scripts/webkitpy/common/net/bugzilla/bugzilla.py:

(BugzillaQueries._parse_attachment_ids_request_query): Removed an annoying log
line that complicated testing.

  • Scripts/webkitpy/common/net/statusserver.py: Pass high priority items separately.
  • Scripts/webkitpy/tool/bot/feeders.py: For commit queue, split high and regular

priority items into separate lists.

  • Scripts/webkitpy/common/net/statusserver_mock.py:
  • Scripts/webkitpy/tool/bot/feeders_unittest.py:
  • Scripts/webkitpy/tool/commands/queues_unittest.py:

Updated tests.

Location:
trunk/Tools
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r174246 r174248  
     12014-10-02  Alexey Proskuryakov  <ap@apple.com>
     2
     3        update-work-items should never delete items
     4        https://bugs.webkit.org/show_bug.cgi?id=137366
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        As we don't just replace the whole list any more, indicate which items are high
     9        priority, and which are not. Hight priority ones will be prepended to the queue,
     10        others will be appended.
     11
     12        This creates a bit of unfairness in that high priority item queue becomes a LIFO.
     13        But hopefully we will never have many rollouts competing like that.
     14
     15        * QueueStatusServer/app.yaml: Update version.
     16
     17        * QueueStatusServer/handlers/updateworkitems.py: Never remove items. Pass high
     18        priority and regular items separately. Removed some error handling that used to
     19        end up in returning status 500 - an uncaught exception does that automatically.
     20
     21        * QueueStatusServer/main.py: Removed unnecessary regexps from URL matching code.
     22
     23        * QueueStatusServer/model/workitems.py: Added a way to add high priority items.
     24
     25        * QueueStatusServer/templates/updateworkitems.html: Added a field for high
     26        priority items.
     27
     28        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
     29        (BugzillaQueries._parse_attachment_ids_request_query): Removed an annoying log
     30        line that complicated testing.
     31
     32        * Scripts/webkitpy/common/net/statusserver.py: Pass high priority items separately.
     33
     34        * Scripts/webkitpy/tool/bot/feeders.py: For commit queue, split high and regular
     35        priority items into separate lists.
     36
     37        * Scripts/webkitpy/common/net/statusserver_mock.py:
     38        * Scripts/webkitpy/tool/bot/feeders_unittest.py:
     39        * Scripts/webkitpy/tool/commands/queues_unittest.py:
     40        Updated tests.
     41
    1422014-10-02  Brendan Long  <b.long@cablelabs.com>
    243

  • trunk/Tools/QueueStatusServer/app.yaml

    r174158 r174248  
    11application: webkit-queues
    2 version: 174158 # Bugzilla bug ID of last major change
     2version: 174248 # Bugzilla bug ID of last major change
    33runtime: python
    44api_version: 1

  • trunk/Tools/QueueStatusServer/handlers/updateworkitems.py

    r140513 r174248  
    11# Copyright (C) 2013 Google Inc. All rights reserved.
     2# Copyright (C) 2014 Apple Inc. All rights reserved.
    23#
    34# Redistribution and use in source and binary forms, with or without
    … …  
    4344
    4445    def _parse_work_items_string(self, items_string):
    45         try:
    46             item_strings = items_string.split(" ") if items_string else []
    47             return map(int, item_strings)
    48         except ValueError:
    49             return None
     46        item_strings = items_string.split(" ") if items_string else []
     47        return map(int, item_strings)
    5048
    51     def _update_work_items_from_request(self, work_items):
     49    def _work_items_from_request(self):
     50        high_priority_items_string = self.request.get("high_priority_items")
    5251        items_string = self.request.get("work_items")
    53         new_work_items = self._parse_work_items_string(items_string)
    54         if new_work_items == None:
    55             self.response.out.write("Failed to parse work items: %s" % items_string)
    56             return False
    57         work_items.item_ids = new_work_items
    58         work_items.date = datetime.utcnow()
    59         return True
     52        high_priority_work_items = self._parse_work_items_string(high_priority_items_string)
     53        work_items = self._parse_work_items_string(items_string)
     54        return high_priority_work_items, work_items
    6055
    6156    def _queue_from_request(self):
    … …  
    7267            self.response.set_status(500)
    7368            return
    74         work_items = queue.work_items()
    75         old_items = set(work_items.item_ids)
    7669
    77         success = self._update_work_items_from_request(work_items)
    78         if not success:
    79             self.response.set_status(500)
    80             return
    81         new_items = set(work_items.item_ids)
    82         work_items.put()
     70        high_priority_items, items = self._work_items_from_request()
    8371
    84         for work_item in new_items - old_items:
     72        # Add items that are not currently in the work queue. Never remove any items,
     73        # as that should be done by the queue, feeder only adds them.
     74        added_items = queue.work_items().add_work_items(high_priority_items, items)
     75
     76        for work_item in added_items:
    8577            RecordPatchEvent.added(work_item, queue.name())
    86         for work_item in old_items - new_items:
    87             RecordPatchEvent.stopped(work_item, queue.name())

  • trunk/Tools/QueueStatusServer/main.py

    r174015 r174248  
    6464    (r'/patch-status/(.*)/(.*)', PatchStatus),
    6565    (r'/patch/(.*)', Patch),
    66     (r'/submit-to-ews', SubmitToEWS),
     66    ('/submit-to-ews', SubmitToEWS),
    6767    (r'/results/(.*)', ShowResults),
    6868    (r'/status-bubble/(.*)', StatusBubble),
    … …  
    7474    (r'/queue-status-json/(.*)', QueueStatusJSON),
    7575    (r'/next-patch/(.*)', NextPatch),
    76     (r'/release-patch', ReleasePatch),
    77     (r'/release-lock', ReleaseLock),
     76    ('/release-patch', ReleasePatch),
     77    ('/release-lock', ReleaseLock),
    7878    ('/update-status', UpdateStatus),
    7979    ('/update-work-items', UpdateWorkItems),

  • trunk/Tools/QueueStatusServer/model/workitems.py

    r173979 r174248  
    11# Copyright (C) 2010 Google Inc. All rights reserved.
     2# Copyright (C) 2014 Apple Inc. All rights reserved.
    23#
    34# Redistribution and use in source and binary forms, with or without
    … …  
    5455
    5556    @staticmethod
    56     def _unguarded_add(key, attachment_id):
     57    def _unguarded_add(key, high_priority_items, items):
    5758        work_items = db.get(key)
    58         if attachment_id in work_items.item_ids:
    59             return
    60         work_items.item_ids.append(attachment_id)
     59        added_items = []
     60        for item in high_priority_items[::-1]:
     61            if item in work_items.item_ids:
     62                continue
     63            work_items.item_ids.insert(0, item)
     64            added_items.insert(0, item)
     65        for item in items:
     66            if item in work_items.item_ids:
     67                continue
     68            work_items.item_ids.append(item)
     69            added_items.append(item)
    6170        work_items.put()
     71        return added_items
    6272
    6373    # Because this uses .key() self.is_saved() must be True or this will throw NotSavedError.
    6474    def add_work_item(self, attachment_id):
    65         db.run_in_transaction(self._unguarded_add, self.key(), attachment_id)
     75        db.run_in_transaction(self._unguarded_add, self.key(), [], [attachment_id])
     76
     77    def add_work_items(self, high_priority_items, items):
     78        return db.run_in_transaction(self._unguarded_add, self.key(), high_priority_items, items)
    6679
    6780    @staticmethod

  • trunk/Tools/QueueStatusServer/templates/updateworkitems.html

    r59534 r174248  
    22Update work items for a queue: <input name="queue_name">
    33 <div>
    4      Work Items:
    5     <input name="work_items">
     4     Work items: <input name="work_items">
     5     High priority work items: <input name="high_priority_work_items">
    66 </div>
    77 <div><input type="submit" value="Update Work Items"></div>

  • trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py

    r170639 r174248  
    205205            date_tag = row.find("td", text=date_format)
    206206            if date_tag and datetime.strptime(date_format.search(date_tag).group(0), "%Y-%m-%d %H:%M") < since:
    207                 _log.info("Patch is old: %d (%s)" % (patch_id, date_tag))
    208207                continue
    209208            patch_ids.append(patch_id)

  • trunk/Tools/Scripts/webkitpy/common/net/statusserver.py

    r173979 r174248  
    101101        return self._browser.submit().read()
    102102
    103     def _post_work_items_to_server(self, queue_name, work_items):
     103    def _post_work_items_to_server(self, queue_name, high_priority_work_items, work_items):
    104104        update_work_items_url = "%s/update-work-items" % self.url
    105105        self._browser.open(update_work_items_url)
    … …  
    108108        work_items = map(unicode, work_items)  # .join expects strings
    109109        self._browser["work_items"] = " ".join(work_items)
     110        high_priority_work_items = map(unicode, high_priority_work_items)
     111        self._browser["high_priority_work_items"] = " ".join(high_priority_work_items)
    110112        return self._browser.submit().read()
    111113
    … …  
    150152        return NetworkTransaction(convert_404_to_None=True).run(lambda: self._post_release_lock(queue_name, patch))
    151153
    152     def update_work_items(self, queue_name, work_items):
    153         _log.debug("Recording work items: %s for %s" % (work_items, queue_name))
    154         return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, work_items))
     154    def update_work_items(self, queue_name, high_priority_work_items, work_items):
     155        _log.debug("Recording work items: %s for %s" % (high_priority_work_items + work_items, queue_name))
     156        return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, high_priority_work_items, work_items))
    155157
    156158    def update_status(self, queue_name, status, patch=None, results_file=None):

  • trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py

    r174009 r174248  
    5656        _log.info("MOCK: release_lock: %s %s" % (queue_name, patch.id()))
    5757
    58     def update_work_items(self, queue_name, work_items):
     58    def update_work_items(self, queue_name, high_priority_work_items, work_items):
    5959        self._work_items = work_items
    60         _log.info("MOCK: update_work_items: %s %s" % (queue_name, work_items))
     60        _log.info("MOCK: update_work_items: %s %s" % (queue_name, high_priority_work_items + work_items))
    6161
    6262    def submit_to_ews(self, patch_id):

  • trunk/Tools/Scripts/webkitpy/tool/bot/feeders.py

    r170637 r174248  
    5151        self.committer_validator = CommitterValidator(self._tool)
    5252
    53     def _update_work_items(self, item_ids):
    54         # FIXME: This is the last use of update_work_items, the commit-queue
    55         # should move to feeding patches one at a time like the EWS does.
    56         self._tool.status_server.update_work_items(self.queue_name, item_ids)
    57         _log.info("Feeding %s items %s" % (self.queue_name, item_ids))
    58 
    5953    def feed(self):
    6054        patches = self._validate_patches()
    6155        patches = self._patches_with_acceptable_review_flag(patches)
    6256        patches = sorted(patches, self._patch_cmp)
    63         patch_ids = [patch.id() for patch in patches]
    64         self._update_work_items(patch_ids)
     57
     58        high_priority_item_ids = [patch.id() for patch in patches if patch.is_rollout()]
     59        item_ids = [patch.id() for patch in patches if not patch.is_rollout()]
     60
     61        _log.info("Feeding %s high priority items %s, regular items %s" % (self.queue_name, high_priority_item_ids, item_ids))
     62        self._tool.status_server.update_work_items(self.queue_name, high_priority_item_ids, item_ids)
    6563
    6664    def _patches_for_bug(self, bug_id):
    … …  
    7876
    7977    def _patch_cmp(self, a, b):
    80         # Sort first by is_rollout, then by attach_date.
    81         # Reversing the order so that is_rollout is first.
    82         rollout_cmp = cmp(b.is_rollout(), a.is_rollout())
    83         if rollout_cmp != 0:
    84             return rollout_cmp
    8578        return cmp(a.attach_date(), b.attach_date())
    8679

  • trunk/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py

    r174136 r174248  
    3838class FeedersTest(unittest.TestCase):
    3939    def test_commit_queue_feeder(self):
     40        self.maxDiff = None
    4041        feeder = CommitQueueFeeder(MockTool())
    4142        expected_logs = """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)
    … …  
    4647
    4748- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.'
     49Feeding commit-queue high priority items [10005], regular items [10000]
    4850MOCK: update_work_items: commit-queue [10005, 10000]
    49 Feeding commit-queue items [10005, 10000]
    5051"""
    5152        OutputCapture().assert_outputs(self, feeder.feed, expected_logs=expected_logs)
    … …  
    5657        attachment.attach_date = lambda: attach_date
    5758        return attachment
    58 
    59     def test_patch_cmp(self):
    60         long_ago_date = datetime(1900, 1, 21)
    61         recent_date = datetime(2010, 1, 21)
    62         attachment1 = self._mock_attachment(is_rollout=False, attach_date=recent_date)
    63         attachment2 = self._mock_attachment(is_rollout=False, attach_date=long_ago_date)
    64         attachment3 = self._mock_attachment(is_rollout=True, attach_date=recent_date)
    65         attachment4 = self._mock_attachment(is_rollout=True, attach_date=long_ago_date)
    66         attachments = [attachment1, attachment2, attachment3, attachment4]
    67         expected_sort = [attachment4, attachment3, attachment2, attachment1]
    68         queue = CommitQueueFeeder(MockTool())
    69         attachments.sort(queue._patch_cmp)
    70         self.assertEqual(attachments, expected_sort)
    7159
    7260    def test_patches_with_acceptable_review_flag(self):

  • trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py

    r174009 r174248  
    129129class FeederQueueTest(QueuesTest):
    130130    def test_feeder_queue(self):
     131        self.maxDiff = None
    131132        queue = TestFeederQueue()
    132133        tool = MockTool(log_executive=True)
    … …  
    140141
    141142- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.'
     143Feeding commit-queue high priority items [10005], regular items [10000]
    142144MOCK: update_work_items: commit-queue [10005, 10000]
    143 Feeding commit-queue items [10005, 10000]
    144145Feeding EWS (1 r? patch, 1 new)
    145146MOCK: submit_to_ews: 10002
Note: See TracChangeset for help on using the changeset viewer.