Merge lp:~bac/charms/precise/juju-gui/support-constraints into lp:~juju-gui/charms/precise/juju-gui/trunk

Proposed by Brad Crittenden
Status: Merged
Merged at revision: 164
Proposed branch: lp:~bac/charms/precise/juju-gui/support-constraints
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 38 lines (+17/-1)
2 files modified
server/guiserver/bundles/utils.py (+15/-1)
server/guiserver/tests/bundles/test_utils.py (+2/-0)
To merge this branch: bzr merge lp:~bac/charms/precise/juju-gui/support-constraints
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+206006@code.launchpad.net

Description of the change

Support for juju core constraints

All juju core constraints should be supported by Juju GUI.
See https://juju.ubuntu.com/docs/reference-constraints.html
The 'tags' constraint was not implemented as it is only supported for MaaS
deployments.

https://codereview.appspot.com/61510051/

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Reviewers: mp+206006_code.launchpad.net,

Message:
Please take a look.

Description:
Support for juju core constraints

All juju core constraints should be supported by Juju GUI.
See https://juju.ubuntu.com/docs/reference-constraints.html
The 'tags' constraint was not implemented as it is only supported for
MaaS
deployments.

https://code.launchpad.net/~bac/charms/precise/juju-gui/support-constraints/+merge/206006

(do not edit description out of merge proposal)

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

Affected files (+14, -1 lines):
   [revision details]
   server/guiserver/bundles/utils.py
   server/guiserver/tests/bundles/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: server/guiserver/bundles/utils.py
=== modified file 'server/guiserver/bundles/utils.py'
--- server/guiserver/bundles/utils.py 2013-11-27 19:25:54 +0000
+++ server/guiserver/bundles/utils.py 2014-02-12 13:55:47 +0000
@@ -39,7 +39,16 @@
  COMPLETED = 'completed'
  # Define a sequence of allowed constraints to be used in the process of
  # preparing the bundle object. See the _prepare_constraints function below.
-ALLOWED_CONSTRAINTS = ('arch', 'cpu-cores', 'cpu-power', 'mem')
+ALLOWED_CONSTRAINTS = (
+ 'arch',
+ 'container',
+ 'cpu-cores',
+ 'cpu-power',
+ 'mem',
+ 'root-disk',
+ # tags are supported by MaaS only so they are not currently supported.
+ # 'tags',
+)

  def create_change(deployment_id, status, queue=None, error=None):

Index: server/guiserver/tests/bundles/test_utils.py
=== modified file 'server/guiserver/tests/bundles/test_utils.py'
--- server/guiserver/tests/bundles/test_utils.py 2013-11-27 19:28:03 +0000
+++ server/guiserver/tests/bundles/test_utils.py 2014-02-12 13:55:47 +0000
@@ -278,6 +278,8 @@
              'cpu-cores': 4,
              'cpu-power': 2,
              'mem': 2000,
+ 'root-disk': '1G',
+ 'container': 'lxc',
          }
          self.assertEqual(constraints,
utils._prepare_constraints(constraints))

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

LGTM with just a minor comment.
Thank you!

https://codereview.appspot.com/61510051/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/61510051/diff/1/server/guiserver/bundles/utils.py#newcode49
server/guiserver/bundles/utils.py:49:
Could you please expand on why we do not support MAAS specific
constraints?

https://codereview.appspot.com/61510051/

165. By Brad Crittenden

Further document the decision to leave out tag constraints.

166. By Brad Crittenden

Further document the decision to leave out tag constraints.

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

*** Submitted:

Support for juju core constraints

All juju core constraints should be supported by Juju GUI.
See https://juju.ubuntu.com/docs/reference-constraints.html
The 'tags' constraint was not implemented as it is only supported for
MaaS
deployments.

R=frankban
CC=
https://codereview.appspot.com/61510051

https://codereview.appspot.com/61510051/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'server/guiserver/bundles/utils.py'
2--- server/guiserver/bundles/utils.py 2013-11-27 19:25:54 +0000
3+++ server/guiserver/bundles/utils.py 2014-02-12 19:11:43 +0000
4@@ -39,7 +39,21 @@
5 COMPLETED = 'completed'
6 # Define a sequence of allowed constraints to be used in the process of
7 # preparing the bundle object. See the _prepare_constraints function below.
8-ALLOWED_CONSTRAINTS = ('arch', 'cpu-cores', 'cpu-power', 'mem')
9+ALLOWED_CONSTRAINTS = (
10+ 'arch',
11+ 'container',
12+ 'cpu-cores',
13+ 'cpu-power',
14+ 'mem',
15+ 'root-disk',
16+ # XXX: BradCrittenden 2014-02-12:
17+ # tags are supported by MaaS only so they are not currently implemented.
18+ # It is unclear whether the GUI should support them or not so they are
19+ # being left out for now.
20+ # Also, tags are a comma-separated, which would clash with the currently
21+ # broken constraint parsing in the GUI.
22+ # 'tags',
23+)
24
25
26 def create_change(deployment_id, status, queue=None, error=None):
27
28=== modified file 'server/guiserver/tests/bundles/test_utils.py'
29--- server/guiserver/tests/bundles/test_utils.py 2013-11-27 19:28:03 +0000
30+++ server/guiserver/tests/bundles/test_utils.py 2014-02-12 19:11:43 +0000
31@@ -278,6 +278,8 @@
32 'cpu-cores': 4,
33 'cpu-power': 2,
34 'mem': 2000,
35+ 'root-disk': '1G',
36+ 'container': 'lxc',
37 }
38 self.assertEqual(constraints, utils._prepare_constraints(constraints))
39

Subscribers

People subscribed via source and target branches