Bug 29307 - commit-queue needs web-based status reporting
Summary: commit-queue needs web-based status reporting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-16 12:30 PDT by Eric Seidel (no email)
Modified: 2009-09-24 13:41 PDT (History)
3 users (show)

See Also:


Attachments
First pass implementation (20.28 KB, patch)
2009-09-17 18:00 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Fix per evan's suggestions (20.23 KB, patch)
2009-09-17 18:37 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Fixed queued bugs url (20.18 KB, patch)
2009-09-18 15:06 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Unify ChangeLogs (19.91 KB, patch)
2009-09-22 15:30 PDT, Eric Seidel (no email)
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-09-16 12:30:07 PDT
commit-queue needs web-based status reporting

Folks need to be able to tell if the commit-queue is running or not, how many patches are in the queue, etc.

A simple web app should suffice.  I'll code something up.
Comment 1 Eric Seidel (no email) 2009-09-17 18:00:47 PDT
Created attachment 39740 [details]
First pass implementation
Comment 2 Eric Seidel (no email) 2009-09-17 18:01:38 PDT
Adding folks with python/appengine familiarity.
Comment 3 Evan Martin 2009-09-17 18:13:45 PDT
Comment on attachment 39740 [details]
First pass implementation

> +    <div id="current_status">
> +        {{ last_status.message|escape|webkit_linkify }}
> +        <div id="last_status_date">As of {{ last_status.date|timesince }} ago</div>
> +    </div>

Djangos since 1.0 autoescape {{}} bits; you only get the old one on appengine by default
for backwards compat.

from google.appengine.dist import use_library
use_library('django', '1.1')

> +class QueueStatus(db.Model):
> +    author = db.UserProperty()
> +    active_bug_id = db.IntegerProperty()
> +    active_patch_id = db.IntegerProperty()
> +    message = db.StringProperty(multiline=True)

Maybe db.Text() is better for this?

> +    date = db.DateTimeProperty(auto_now_add=True)

> +        path = os.path.join(os.path.dirname(__file__), 'index.html')
> +        self.response.out.write(template.render(path, template_values))

Consider factoring this repeated bit into a helper function.

> +    def post(self):
> +        queue_status = QueueStatus()
> +
> +        if users.get_current_user():
> +            queue_status.author = users.get_current_user()
> +
> +        queue_status.active_bug_id = self._int_from_request('bug_id')
> +        queue_status.active_patch_id = self._int_from_request('patch_id')
> +        queue_status.message = self.request.get('status')
> +        queue_status.put()
> +        self.redirect('/')

I guess this is XSRFable.
Comment 4 Eric Seidel (no email) 2009-09-17 18:34:55 PDT
(In reply to comment #3)
> from google.appengine.dist import use_library
> use_library('django', '1.1')

Thanks!

> Maybe db.Text() is better for this?

StringProperty seems more when I want.
http://code.google.com/appengine/docs/python/datastore/typesandpropertyclasses.html#StringProperty

> I guess this is XSRFable.

This app doesn't use authentication, so that's OK. :)  Eventually it might display the name of who set the status, but making the commit-queue use a Google cookie when posting is more trouble than I want to deal with.
Comment 5 Eric Seidel (no email) 2009-09-17 18:37:04 PDT
Created attachment 39744 [details]
Fix per evan's suggestions
Comment 6 Eric Seidel (no email) 2009-09-18 15:06:47 PDT
Created attachment 39795 [details]
Fixed queued bugs url
Comment 7 Eric Seidel (no email) 2009-09-22 15:30:42 PDT
Created attachment 39952 [details]
Unify ChangeLogs
Comment 8 David Levin 2009-09-22 16:21:11 PDT
Comment on attachment 39952 [details]
Unify ChangeLogs

Just a few nits to fix on landing.


> diff --git a/WebKitTools/CommitQueueStatus/filters/webkit_extras.py b/WebKitTools/CommitQueueStatus/filters/webkit_extras.py
> +# Copyright (c) 2009, Google Inc. All rights reserved.

Ideally (C) and no comma after the year.

> +bug_regexp = re.compile("bug (?P<bug_id>\d+)")

Put r in front of the quotes. (Fortunately \d doesn't map to a string escape but best to put use r"" for this anyway.)

> +patch_regexp = re.compile("patch (?P<patch_id>\d+)")

Put r in front of the quotes.

> +
> +@stringfilter
> +def webkit_linkify(value):
> +    value = bug_regexp.sub('<a href="http://webkit.org/b/\g<bug_id>">bug \g<bug_id></a>', value)

Put r in front of the single quote (to make to clear that the \g isn't a string escape).

> +    value = patch_regexp.sub('<a href="https://bugs.webkit.org/attachment.cgi?id=\g<patch_id>&action=prettypatch">patch \g<patch_id></a>', value)

Put r in front of the single quote.


> diff --git a/WebKitTools/CommitQueueStatus/queue_status.py b/WebKitTools/CommitQueueStatus/queue_status.py
> +# Copyright (c) 2009, Google Inc. All rights reserved.

Ideally (C) and no comma after the year.


> diff --git a/WebKitTools/Scripts/modules/statusbot.py b/WebKitTools/Scripts/modules/statusbot.py

> +# Copyright (c) 2009, Google Inc. All rights reserved.

Ideally (C) and no comma after the year.
Comment 9 Eric Seidel (no email) 2009-09-24 13:41:10 PDT
Committed r48730: <http://trac.webkit.org/changeset/48730>