Make patch release explicit and not a magic part of "retry" status
Created attachment 71177 [details] Patch
Comment on attachment 71177 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71177&action=review > WebKitTools/Scripts/webkitpy/common/net/statusserver.py:116 > + def release_work_item(self, queue_name, patch): > + _log.debug("Releasing work item %s from %s" % (patch.id(), queue_name)) > + release_patch_url = "%s/release-patch/%s" % (self.url, queue_name) > + self._browser.open(release_patch_url) > + self._browser.select_form(name="release_patch") > + self._browser["attachment_id"] = patch.id() > + self._browser.submit() We need a network transaction for writing to appengine > WebKitTools/Scripts/webkitpy/tool/mocktool.py:551 > + def release_work_item(self, queue_name, patch): > + pass Should we log so we can test that this happens?
Created attachment 71197 [details] Updated per Adam's suggestions
Comment on attachment 71197 [details] Updated per Adam's suggestions View in context: https://bugs.webkit.org/attachment.cgi?id=71197&action=review > WebKitTools/QueueStatusServer/handlers/nextpatch.py:39 > + # FIXME: This should probably be a post, or an explict lock_patch > + # since GET requests shouldn't really modify the datastore. Good point. > WebKitTools/QueueStatusServer/handlers/releasepatch.py:46 > + # FIXME: This queue lookup should be shared between handlers. > + queue = Queue.queue_with_name(queue_name) > + if not queue: > + self.error(404) > + return Yeah, feels like a base class to me. > WebKitTools/QueueStatusServer/handlers/releasepatch.py:53 > + if not latest_status: > + self.error(404) > + return When would this ever happen? > WebKitTools/QueueStatusServer/handlers/releasepatch.py:56 > + # Ideally we should use a transaction for the calls to > + # WorkItems and ActiveWorkItems. Yeah, I don't know if this will be a problem. I guess we'll find out. We could ask some AppEngine experts. I suspect the concurrency model doesn't give you a total ordering between transactions. I asked Mihai, but he didn't know.
Comment on attachment 71197 [details] Updated per Adam's suggestions Actually, this requires a queue reboot, right?
I mean, any queue change does. If the queues don't reboot, nothing will really change except that retries will be delayed until the lock expires. I'll land it manually and reboot my local queue.
You should be able to reboot mine. Look for a command window that's running the commit-queue. The command I use is start-commit-queue.sh. It should be in the bash history.
Committed r70088: <http://trac.webkit.org/changeset/70088>