Merge lp://staging/~frankban/charms/precise/juju-gui/support-default-charm-icon into lp://staging/~juju-gui/charms/precise/juju-gui/trunk

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 176
Proposed branch: lp://staging/~frankban/charms/precise/juju-gui/support-default-charm-icon
Merge into: lp://staging/~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 287 lines (+146/-21)
5 files modified
revision (+1/-1)
server/guiserver/__init__.py (+1/-1)
server/guiserver/apps.py (+6/-2)
server/guiserver/handlers.py (+78/-15)
server/guiserver/tests/test_handlers.py (+60/-2)
To merge this branch: bzr merge lp://staging/~frankban/charms/precise/juju-gui/support-default-charm-icon
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+214985@code.staging.launchpad.net

Description of the change

Make the GUI server redirect to the default icon.

Create a specialized proxy handler for handling
Juju HTTP API requests. In this subclass, handle
the case a request is for a local charm icon
that cannot be found on the Juju server.

Tests: `make unittest`.

QA:
- `juju bootstrap`;
- from the branch root, run `make deploy`;
- wait for the GUI service to be ready;
- switch to the trunk branch:
  `juju set juju-gui juju-gui-source=develop`
- wait for the GUI to be ready;
- deploy local charms including an icon:
  you should see the icons are correctly displayed in the
  service blocks and inspector header;
- deploy a local charm not including an icon:
  you should see the fallback icon displayed both in
  the service block and the inspector;
- destroy the environment, done.

https://codereview.appspot.com/86100043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+214985_code.launchpad.net,

Message:
Please take a look.

Description:
Make the GUI server redirect to the default icon.

Create a specialized proxy handler for handling
Juju HTTP API requests. In this subclass, handle
the case a request is for a local charm icon
that cannot be found on the Juju server.

Tests: `make unittest`.

QA:
- `juju bootstrap`;
- from the branch root, run `make deploy`;
- wait for the GUI service to be ready;
- switch to the trunk branch:
   `juju set juju-gui juju-gui-source=develop`
- wait for the GUI to be ready;
- deploy local charms including an icon:
   you should see the icons are correctly displayed in the
   service blocks and inspector header;
- deploy a local charm not including an icon:
   you should see the fallback icon displayed both in
   the service block and the inspector;
- destroy the environment, done.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/support-default-charm-icon/+merge/214985

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/86100043/

Affected files (+148, -21 lines):
   A [revision details]
   M revision
   M server/guiserver/__init__.py
   M server/guiserver/apps.py
   M server/guiserver/handlers.py
   M server/guiserver/tests/test_handlers.py

Revision history for this message
Brad Crittenden (bac) wrote :

Code LGTM

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py
File server/guiserver/handlers.py (right):

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py#newcode255
server/guiserver/handlers.py:255: # Handle POST requests the same way
GET ones are handled.
s/ones/requests/ -- just reads better.

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py#newcode320
server/guiserver/handlers.py:320: Override to handle the case a when a
charm icon is not found.
s/a when/when/ or /for when/?

https://codereview.appspot.com/86100043/

180. By Francesco Banconi

Changes as per review.

Revision history for this message
Francesco Banconi (frankban) wrote :

Please take a look.

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py
File server/guiserver/handlers.py (right):

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py#newcode255
server/guiserver/handlers.py:255: # Handle POST requests the same way
GET ones are handled.
On 2014/04/09 16:32:42, bac wrote:
> s/ones/requests/ -- just reads better.

Done.

https://codereview.appspot.com/86100043/diff/1/server/guiserver/handlers.py#newcode320
server/guiserver/handlers.py:320: Override to handle the case a when a
charm icon is not found.
On 2014/04/09 16:32:42, bac wrote:
> s/a when/when/ or /for when/?

Done.

https://codereview.appspot.com/86100043/

Revision history for this message
Brad Crittenden (bac) wrote :
Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Make the GUI server redirect to the default icon.

Create a specialized proxy handler for handling
Juju HTTP API requests. In this subclass, handle
the case a request is for a local charm icon
that cannot be found on the Juju server.

Tests: `make unittest`.

QA:
- `juju bootstrap`;
- from the branch root, run `make deploy`;
- wait for the GUI service to be ready;
- switch to the trunk branch:
   `juju set juju-gui juju-gui-source=develop`
- wait for the GUI to be ready;
- deploy local charms including an icon:
   you should see the icons are correctly displayed in the
   service blocks and inspector header;
- deploy a local charm not including an icon:
   you should see the fallback icon displayed both in
   the service block and the inspector;
- destroy the environment, done.

R=bac
CC=
https://codereview.appspot.com/86100043

https://codereview.appspot.com/86100043/

Revision history for this message
Francesco Banconi (frankban) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches