Merge lp:~bac/charms/precise/juju-gui/expose-cookies into lp:~juju-gui/charms/precise/juju-gui/trunk

Proposed by Brad Crittenden
Status: Merged
Merged at revision: 69
Proposed branch: lp:~bac/charms/precise/juju-gui/expose-cookies
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 108 lines (+28/-4)
6 files modified
config.yaml (+10/-0)
config/config.js.template (+1/-0)
hooks/backend.py (+2/-1)
hooks/utils.py (+2/-1)
revision (+1/-1)
tests/test_utils.py (+12/-1)
To merge this branch: bzr merge lp:~bac/charms/precise/juju-gui/expose-cookies
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+170118@code.launchpad.net

Description of the change

Expose 'use-analytics' via the charm.

https://codereview.appspot.com/10395043/

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

Reviewers: mp+170118_code.launchpad.net,

Message:
Please take a look.

Description:
Expose 'use-analytics' via the charm.

https://code.launchpad.net/~bac/charms/precise/juju-gui/expose-cookies/+merge/170118

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M config.yaml
   M config/config.js.template
   M hooks/backend.py
   M hooks/utils.py
   M revision
   M tests/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: config.yaml
=== modified file 'config.yaml'
--- config.yaml 2013-06-12 08:59:15 +0000
+++ config.yaml 2013-06-18 15:38:18 +0000
@@ -140,3 +140,13 @@
        'add-apt-repository'.
      type: string
      default: ppa:juju-gui-charmers/stable
+ use-analytics:
+ description: |
+ The team developing the Juju GUI benefits from understanding how
+ different deployments use the tool. By enabling this setting,
+ anonymized usage data is reported back using Google Analytics. The
type
+ of data collected includes the charms that are deployed and the
number
+ of units per service. Use of analytics is optional but we hope you
will
+ allow us to improve our tool based on your experience.
+ type: boolean
+ default: true

Index: revision
=== modified file 'revision'
--- revision 2013-06-12 09:07:29 +0000
+++ revision 2013-06-18 15:39:35 +0000
@@ -1,1 +1,1 @@
-53
+54

Index: config/config.js.template
=== modified file 'config/config.js.template'
--- config/config.js.template 2013-04-26 21:05:03 +0000
+++ config/config.js.template 2013-06-18 15:38:18 +0000
@@ -22,5 +22,6 @@
    apiBackend: {{api_backend}}, // Value can be 'python' or 'go'.
    readOnly: {{readonly}},
    sandbox: {{sandbox}},
+ useAnalytics: {{use_analytics}},
    login_help: {{login_help}}
  };

Index: hooks/backend.py
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2013-06-11 14:04:04 +0000
+++ hooks/backend.py 2013-06-18 15:38:18 +0000
@@ -108,7 +108,8 @@
              config['juju-gui-console-enabled'], config['login-help'],
              config['read-only'], config['staging'],
config['ssl-cert-path'],
              config['charmworld-url'], config['serve-tests'],
- secure=config['secure'], sandbox=config['sandbox'])
+ secure=config['secure'], sandbox=config['sandbox'],
+ use_analytics=config['use-analytics'])
          charmhelpers.open_port(80)
          charmhelpers.open_port(443)

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2013-06-12 08:59:15 +0000
+++ hooks/utils.py 2013-06-18 15:38:18 +0000
@@ -360,7 +360,7 @@
  def start_gui(
          console_enabled, login_help, readonly, in_staging, ssl_cert_path,
          charmworld_url, serve_tests,
haproxy_path='/etc/haproxy/haproxy.cfg',
- config_js_path=None, secure=True, sandbox=False):
...

Read more...

Revision history for this message
Nicola Larosa (teknico) wrote :

LGTM, where "G" means "flawless". :-)

https://codereview.appspot.com/10395043/

Revision history for this message
Madison Scott-Clary (makyo) wrote :

LGTM - thanks for getting this plugged in

https://codereview.appspot.com/10395043/

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

*** Submitted:

Expose 'use-analytics' via the charm.

R=teknico, matthew.scott
CC=
https://codereview.appspot.com/10395043

https://codereview.appspot.com/10395043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2013-06-12 08:59:15 +0000
+++ config.yaml 2013-06-18 15:47:28 +0000
@@ -140,3 +140,13 @@
140 'add-apt-repository'.140 'add-apt-repository'.
141 type: string141 type: string
142 default: ppa:juju-gui-charmers/stable142 default: ppa:juju-gui-charmers/stable
143 use-analytics:
144 description: |
145 The team developing the Juju GUI benefits from understanding how
146 different deployments use the tool. By enabling this setting,
147 anonymized usage data is reported back using Google Analytics. The type
148 of data collected includes the charms that are deployed and the number
149 of units per service. Use of analytics is optional but we hope you will
150 allow us to improve our tool based on your experience.
151 type: boolean
152 default: true
143153
=== modified file 'config/config.js.template'
--- config/config.js.template 2013-04-26 21:05:03 +0000
+++ config/config.js.template 2013-06-18 15:47:28 +0000
@@ -22,5 +22,6 @@
22 apiBackend: {{api_backend}}, // Value can be 'python' or 'go'.22 apiBackend: {{api_backend}}, // Value can be 'python' or 'go'.
23 readOnly: {{readonly}},23 readOnly: {{readonly}},
24 sandbox: {{sandbox}},24 sandbox: {{sandbox}},
25 useAnalytics: {{use_analytics}},
25 login_help: {{login_help}}26 login_help: {{login_help}}
26};27};
2728
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2013-06-11 14:04:04 +0000
+++ hooks/backend.py 2013-06-18 15:47:28 +0000
@@ -108,7 +108,8 @@
108 config['juju-gui-console-enabled'], config['login-help'],108 config['juju-gui-console-enabled'], config['login-help'],
109 config['read-only'], config['staging'], config['ssl-cert-path'],109 config['read-only'], config['staging'], config['ssl-cert-path'],
110 config['charmworld-url'], config['serve-tests'],110 config['charmworld-url'], config['serve-tests'],
111 secure=config['secure'], sandbox=config['sandbox'])111 secure=config['secure'], sandbox=config['sandbox'],
112 use_analytics=config['use-analytics'])
112 charmhelpers.open_port(80)113 charmhelpers.open_port(80)
113 charmhelpers.open_port(443)114 charmhelpers.open_port(443)
114115
115116
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2013-06-12 08:59:15 +0000
+++ hooks/utils.py 2013-06-18 15:47:28 +0000
@@ -360,7 +360,7 @@
360def start_gui(360def start_gui(
361 console_enabled, login_help, readonly, in_staging, ssl_cert_path,361 console_enabled, login_help, readonly, in_staging, ssl_cert_path,
362 charmworld_url, serve_tests, haproxy_path='/etc/haproxy/haproxy.cfg',362 charmworld_url, serve_tests, haproxy_path='/etc/haproxy/haproxy.cfg',
363 config_js_path=None, secure=True, sandbox=False):363 config_js_path=None, secure=True, sandbox=False, use_analytics=False):
364 """Set up and start the Juju GUI server."""364 """Set up and start the Juju GUI server."""
365 with su('root'):365 with su('root'):
366 run('chown', '-R', 'ubuntu:', JUJU_GUI_DIR)366 run('chown', '-R', 'ubuntu:', JUJU_GUI_DIR)
@@ -400,6 +400,7 @@
400 'protocol': json.dumps(protocol),400 'protocol': json.dumps(protocol),
401 'sandbox': json.dumps(sandbox),401 'sandbox': json.dumps(sandbox),
402 'charmworld_url': json.dumps(charmworld_url),402 'charmworld_url': json.dumps(charmworld_url),
403 'use_analytics': json.dumps(use_analytics),
403 }404 }
404 if config_js_path is None:405 if config_js_path is None:
405 config_js_path = os.path.join(406 config_js_path = os.path.join(
406407
=== modified file 'revision'
--- revision 2013-06-12 09:07:29 +0000
+++ revision 2013-06-18 15:47:28 +0000
@@ -1,1 +1,1 @@
153154
22
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py 2013-06-11 14:04:04 +0000
+++ tests/test_utils.py 2013-06-18 15:47:28 +0000
@@ -637,7 +637,7 @@
637 start_gui(637 start_gui(
638 False, 'This is login help.', True, True, ssl_cert_path,638 False, 'This is login help.', True, True, ssl_cert_path,
639 charmworld_url, True, haproxy_path='haproxy',639 charmworld_url, True, haproxy_path='haproxy',
640 config_js_path='config')640 config_js_path='config', use_analytics=True)
641 haproxy_conf = self.files['haproxy']641 haproxy_conf = self.files['haproxy']
642 self.assertIn('ca-base {0}'.format(ssl_cert_path), haproxy_conf)642 self.assertIn('ca-base {0}'.format(ssl_cert_path), haproxy_conf)
643 self.assertIn('crt-base {0}'.format(ssl_cert_path), haproxy_conf)643 self.assertIn('crt-base {0}'.format(ssl_cert_path), haproxy_conf)
@@ -655,6 +655,7 @@
655 self.assertIn("socket_url: 'wss://", js_conf)655 self.assertIn("socket_url: 'wss://", js_conf)
656 self.assertIn('socket_protocol: "wss"', js_conf)656 self.assertIn('socket_protocol: "wss"', js_conf)
657 self.assertIn('charmworldURL: "http://charmworld.example"', js_conf)657 self.assertIn('charmworldURL: "http://charmworld.example"', js_conf)
658 self.assertIn('useAnalytics: true', js_conf)
658 apache_conf = self.files['juju-gui']659 apache_conf = self.files['juju-gui']
659 self.assertIn('juju-gui/build-', apache_conf)660 self.assertIn('juju-gui/build-', apache_conf)
660 self.assertIn('VirtualHost *:{0}'.format(WEB_PORT), apache_conf)661 self.assertIn('VirtualHost *:{0}'.format(WEB_PORT), apache_conf)
@@ -687,6 +688,16 @@
687 self.assertIn('user: "admin"', js_conf)688 self.assertIn('user: "admin"', js_conf)
688 self.assertIn('password: "admin"', js_conf)689 self.assertIn('password: "admin"', js_conf)
689690
691 def test_start_gui_no_analytics(self):
692 ssl_cert_path = '/tmp/certificates/'
693 charmworld_url = 'http://charmworld.example'
694 start_gui(
695 False, 'This is login help.', False, False, ssl_cert_path,
696 charmworld_url, True, haproxy_path='haproxy',
697 config_js_path='config', use_analytics=False)
698 js_conf = self.files['config']
699 self.assertIn('useAnalytics: false', js_conf)
700
690701
691class TestNpmCache(unittest.TestCase):702class TestNpmCache(unittest.TestCase):
692 """To speed building from a branch we prepopulate the NPM cache."""703 """To speed building from a branch we prepopulate the NPM cache."""

Subscribers

People subscribed via source and target branches