Bug 47909 – Make patch release explicit and not a magic part of "retry" status

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Make patch release explicit and not a magic part of "retry" status

[Eric Seidel (no email)]

Created attachment 71177 [details]
Patch

[Adam Barth]

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?

[Eric Seidel (no email)]

Created attachment 71197 [details]
Updated per Adam's suggestions

[Adam Barth]

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.

[Adam Barth]

Comment on attachment 71197 [details]
Updated per Adam's suggestions

Actually, this requires a queue reboot, right?

[Eric Seidel (no email)]

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.

[Adam Barth]

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.

[Eric Seidel (no email)]

Committed r70088: <http://trac.webkit.org/changeset/70088>