Closed
Bug 672918
Opened 13 years ago
Closed 13 years ago
create basic structure/code for balrog's human admin interface
Categories
(Release Engineering :: General, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: bhearsum)
References
Details
(Whiteboard: [ignore comment #0])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
nthomas
:
review+
catlee
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nthomas
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
Build machines will need to be able to interact with AUS3 when publishing nightly updates and releases. They need to be able to submit a new release, and possibly make changes to the rules. This could be implemented as a REST API, something else over HTTP, or something else altogether.
If implemented over HTTP, this and bug 672916 might be able to share the backend.
Assignee | ||
Comment 1•13 years ago
|
||
I've been working on designing this here: https://wiki.mozilla.org/User:Bhearsum/AUS3/Administration
Assignee: nobody → bhearsum
Assignee | ||
Comment 2•13 years ago
|
||
This patch is a rough take on the structure of the admin interface for AUS3. I've implemented most of the /users API as a way of getting a feel for things.
Currently, web/base.py has an 'app', which imports all of the existing views. If we built multiple frontends (eg, admin + client) we'd probably need a web/admin.py and web/client.py - which would only import the things they want.
I fiddled with different ways of doing HTML generation for awhile: server side with templates, server side without templates, barebones server side + AJAX loading of content. In the end, I settled on server side with templates because it seemed the simplest -- doing a lot of AJAX loading seemed like I was spreading out the building of the pages unnecessarily.
As noted in the comments, the authentication scheme will need to change when we figure out what we're doing in production. It's isolated in the few methods in web/view/base.py, so it shouldn't be a big deal to change.
This page builds on some more additions to db.py, too. Once I finish addressing the review comments in bug 678163 I'll be posting an updated patch there. If you want to have a look at the current state of things, https://github.com/bhearsum/aus3-proto/blob/history/auslib/db.py has it.
Attachment #556663 -
Flags: feedback?(nrthomas)
Attachment #556663 -
Flags: feedback?(catlee)
Comment 3•13 years ago
|
||
Comment on attachment 556663 [details] [diff] [review]
structure + /users implementation
Review of attachment 556663 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good!
Let's get rid of the circular imports (base.py -> permissions.py -> base.py); maybe use a setup() function in permissions.py to set up the routes
Attachment #556663 -
Flags: feedback?(catlee) → feedback+
Comment 4•13 years ago
|
||
Comment on attachment 556663 [details] [diff] [review]
structure + /users implementation
Looks good. BTW, we lost auslib/__init__.py somewhere along the way to the hg copy of aus3-proto.
Attachment #556663 -
Flags: feedback?(nrthomas) → feedback+
Assignee | ||
Updated•13 years ago
|
Whiteboard: [aus3]
Assignee | ||
Comment 5•13 years ago
|
||
Not _too_ much change here. The biggest thing is passing along data_version all over the place to cope with the version checking implemented in bug 678163. The other things in this patch are:
* Small improvements to readability in templates (foo.bar instead of foo['bar'])
* Strip leading slash from permissions to avoid double slashes in form actions (which Flask doesn't deal with very well, apparently)
* Move permission/option parsing into common places
Attachment #556663 -
Attachment is obsolete: true
Attachment #564627 -
Flags: review?(nrthomas)
Attachment #564627 -
Flags: review?(catlee)
Comment 6•13 years ago
|
||
Comment on attachment 564627 [details] [diff] [review]
updated to cope with versioning, a few other small things
Review of attachment 564627 [details] [diff] [review]:
-----------------------------------------------------------------
::: auslib/web/base.py
@@ +1,3 @@
> +from flask import Flask
> +
> +from sqlalchemy import create_engine
not needed?
Attachment #564627 -
Flags: review?(catlee) → review+
Updated•13 years ago
|
Attachment #564627 -
Flags: review?(nrthomas) → review+
Assignee | ||
Comment 7•13 years ago
|
||
I just realized that this work was supposed to go in bug 672916 =\. Updating the summary to reflect what's actually gone on here.
Summary: create AUS3 API for build machines → create human admin interface for AUS3
Whiteboard: [aus3] → [aus3][ignore comment #0]
Assignee | ||
Comment 9•13 years ago
|
||
Comment on attachment 564627 [details] [diff] [review]
updated to cope with versioning, a few other small things
::: auslib/web/base.py
@@ +1,3 @@
> +from flask import Flask
> +
> +from sqlalchemy import create_engine
> not needed?
Landed, and I removed this unneeded import.
Attachment #564627 -
Flags: checked-in+
Assignee | ||
Updated•13 years ago
|
Summary: create human admin interface for AUS3 → create basic structure/code for balrog's human admin interface
Whiteboard: [aus3][ignore comment #0] → [ignore comment #0]
Assignee | ||
Comment 10•13 years ago
|
||
This patch started with just implementing WTForms, but as I started thinking about some of our use cases more it became clear to me that there's going to be cases where we may want to use similar forms or displays of data on multiple different pages. Given that, I decided to factor out things like "create a form for permissions" to a little snippet of jinja2 that can be included onto pages. Those two things are the bulk of this patch. I also:
* Adde some more debugging to AUSTable
* Renamed some tests to something I thought made a bit more sense
* Implemented add/update/delete of permissions, for new and existing users w/ jquery.
* Fix a bug in requirepermission()'s reporting of errors (it used to report all KeyError's as being unable to find "product").
Attachment #605041 -
Flags: review?(nrthomas)
Comment 11•13 years ago
|
||
Comment on attachment 605041 [details] [diff] [review]
rework html/javascript to make it easier to share across multiple pages; use WTForms
Review of attachment 605041 [details] [diff] [review]:
-----------------------------------------------------------------
Some minor things I noticed, still need to tackle the html changes.
::: auslib/db.py
@@ +184,5 @@
>
> @rtype: sqlalchemy.engine.base.ResultProxy
> """
> query = self._selectStatement(**kwargs)
> + log.debug("AUSTable._prepareUpdate: Executing query: '%s'", query)
nit - this is in select()
@@ +212,5 @@
> data = columns.copy()
> if self.versioned:
> data['data_version'] = 1
> + query = self._insertStatement(**data)
> + log.debug("AUSTable._prepareUpdate: Executing query: '%s' with values: %s", query, data)
nit - this is in _prepareInsert()
@@ +274,5 @@
> where = copy(where)
> where.append(self.data_version==old_data_version)
>
> + query = self._deleteStatement(where)
> + log.debug("AUSTable._prepareUpdate: Executing query: '%s'", query)
nit - this is in _prepareDelete()
::: auslib/web/templates/base.html
@@ +1,5 @@
> <!DOCTYPE HTML>
> <html lang='en-US'>
> <head>
> <title>{% block title %}{% endblock %}</title>
> +<link rel='stylesheet' type='text/css' href='{{ url_for('static', filename='ausadmin.css') }}' />
ausadmin.css isn't included in the patch, should it be ?
Assignee | ||
Comment 12•13 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #11)
> Comment on attachment 605041 [details] [diff] [review]
> rework html/javascript to make it easier to share across multiple pages; use
> WTForms
>
> Review of attachment 605041 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Some minor things I noticed, still need to tackle the html changes.
>
> ::: auslib/db.py
> @@ +184,5 @@
> >
> > @rtype: sqlalchemy.engine.base.ResultProxy
> > """
> > query = self._selectStatement(**kwargs)
> > + log.debug("AUSTable._prepareUpdate: Executing query: '%s'", query)
>
> nit - this is in select()
>
> @@ +212,5 @@
> > data = columns.copy()
> > if self.versioned:
> > data['data_version'] = 1
> > + query = self._insertStatement(**data)
> > + log.debug("AUSTable._prepareUpdate: Executing query: '%s' with values: %s", query, data)
>
> nit - this is in _prepareInsert()
>
> @@ +274,5 @@
> > where = copy(where)
> > where.append(self.data_version==old_data_version)
> >
> > + query = self._deleteStatement(where)
> > + log.debug("AUSTable._prepareUpdate: Executing query: '%s'", query)
>
> nit - this is in _prepareDelete()
Whoops! Will fix these up.
> ::: auslib/web/templates/base.html
> @@ +1,5 @@
> > <!DOCTYPE HTML>
> > <html lang='en-US'>
> > <head>
> > <title>{% block title %}{% endblock %}</title>
> > +<link rel='stylesheet' type='text/css' href='{{ url_for('static', filename='ausadmin.css') }}' />
>
> ausadmin.css isn't included in the patch, should it be ?
I haven't actually created it yet, I just threw it in there for future use. I can drop it for now, if you'd like.
Comment 13•13 years ago
|
||
Comment on attachment 605041 [details] [diff] [review]
rework html/javascript to make it easier to share across multiple pages; use WTForms
>diff --git a/auslib/web/templates/snippets/new_permission.html b/auslib/web/templates/snippets/new_permission.html
I was kinda-joking kinda-not about 'snippets' being a dirty word. If 'fragments' is fine by you then lets use that instead.
r+ with the nits on my previous comment.
Attachment #605041 -
Flags: review?(nrthomas) → review+
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #13)
> Comment on attachment 605041 [details] [diff] [review]
> rework html/javascript to make it easier to share across multiple pages; use
> WTForms
>
> >diff --git a/auslib/web/templates/snippets/new_permission.html b/auslib/web/templates/snippets/new_permission.html
>
> I was kinda-joking kinda-not about 'snippets' being a dirty word. If
> 'fragments' is fine by you then lets use that instead.
Absolutely. Let's keep the past in the past =).
And re: your IRC comment about maybe getting someone from webdev to look at it - I think that's a great idea. I'm going to land this anyways, but I'll get someone to look at what we've got so far, and address any feedback they have.
Assignee | ||
Comment 15•13 years ago
|
||
Comment on attachment 605041 [details] [diff] [review]
rework html/javascript to make it easier to share across multiple pages; use WTForms
https://jenkins.mozilla.org/job/Balrog/56/
Attachment #605041 -
Flags: checked-in+
Assignee | ||
Comment 16•13 years ago
|
||
Rhelmer had a look at the current code over IRC yesterday and didn't find anything that needed fixing.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•