Merge lp:~gary/charms/precise/juju-gui/bug1218888 into lp:~juju-gui/charms/precise/juju-gui/trunk

[Gary Poster]

Add support for xz compression

An xz tarball reduces the size of the gui code by 30 to 40 percent. These changes let the charm use xz tarballs. A very simple change to the gui makefile will follow.

To qa, apply this diff to the gui trunk:

=== modified file 'Makefile'
--- Makefile 2013-10-12 01:30:26 +0000
+++ Makefile 2013-10-14 13:43:10 +0000
@@ -95,7 +95,7 @@
 LAUNCHPAD_API_ROOT=staging
 endif
 RELEASE_NAME=juju-gui-$(RELEASE_VERSION)
-RELEASE_FILE=releases/$(RELEASE_NAME).tgz
+RELEASE_FILE=releases/$(RELEASE_NAME).xz
 RELEASE_SIGNATURE=releases/$(RELEASE_NAME).asc
 NPM_CACHE_VERSION=$(BZR_REVNO)
 NPM_CACHE_FILE=$(CURDIR)/releases/npm-cache-$(NPM_CACHE_VERSION).tgz

Then run ``BRANCH_IS_GOOD=1 make distfile``. Copy over the new release to the charm's releases directory. If you compare the two file sizes in the charm's releases directory, the difference should be dramatic.

$ ls -l
-rw-r--r-- 1 gary gary 5076088 Oct 14 09:44 juju-gui-0.10.1+build.1133.xz
-rw-r--r-- 1 gary gary 44840221 Oct 12 20:15 juju-gui-0.10.1.tgz

You can remove the tgz, and then run juju bootstrap and make deploy in the charm root directory. Once it is deployed, you should be able to log in and see the gui as usual, and you should be able to verify the fact that you are using your custom release if you go to /juju-ui/version.js, where you will see something like "var jujuGuiVersionInfo=['unreleased', '1133'];".

Thank you!

https://codereview.appspot.com/14425057/

[Gary Poster (gary)]

Reviewers: mp+190971_code.launchpad.net,

Message:
Please take a look.

Description:
Add support for xz compression

An xz tarball reduces the size of the gui code by 30 to 40 percent.
These changes let the charm use xz tarballs. A very simple change to
the gui makefile will follow.

To qa, apply this diff to the gui trunk:

=== modified file 'Makefile'
--- Makefile 2013-10-12 01:30:26 +0000
+++ Makefile 2013-10-14 13:43:10 +0000
@@ -95,7 +95,7 @@
  LAUNCHPAD_API_ROOT=staging
  endif
  RELEASE_NAME=juju-gui-$(RELEASE_VERSION)
-RELEASE_FILE=releases/$(RELEASE_NAME).tgz
+RELEASE_FILE=releases/$(RELEASE_NAME).xz
  RELEASE_SIGNATURE=releases/$(RELEASE_NAME).asc
  NPM_CACHE_VERSION=$(BZR_REVNO)
  NPM_CACHE_FILE=$(CURDIR)/releases/npm-cache-$(NPM_CACHE_VERSION).tgz

Then run ``BRANCH_IS_GOOD=1 make distfile``. Copy over the new release
to the charm's releases directory. If you compare the two file sizes in
the charm's releases directory, the difference should be dramatic.

$ ls -l
-rw-r--r-- 1 gary gary 5076088 Oct 14 09:44
juju-gui-0.10.1+build.1133.xz
-rw-r--r-- 1 gary gary 44840221 Oct 12 20:15 juju-gui-0.10.1.tgz

You can remove the tgz, and then run juju bootstrap and make deploy in
the charm root directory. Once it is deployed, you should be able to
log in and see the gui as usual, and you should be able to verify the
fact that you are using your custom release if you go to
/juju-ui/version.js, where you will see something like "var
jujuGuiVersionInfo=['unreleased', '1133'];".

Thank you!

https://code.launchpad.net/~gary/charms/precise/juju-gui/bug1218888/+merge/190971

(do not edit description out of merge proposal)

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

Affected files (+84, -37 lines):
   A [revision details]
   M hooks/utils.py
   M tests/test_utils.py

[Brad Crittenden (bac)]

LGTM and QA ok. Thanks.

https://codereview.appspot.com/14425057/

[Gary Poster (gary)]

*** Submitted:

Add support for xz compression

An xz tarball reduces the size of the gui code by 30 to 40 percent.
These changes let the charm use xz tarballs. A very simple change to
the gui makefile will follow.

To qa, apply this diff to the gui trunk:

=== modified file 'Makefile'
--- Makefile 2013-10-12 01:30:26 +0000
+++ Makefile 2013-10-14 13:43:10 +0000
@@ -95,7 +95,7 @@
  LAUNCHPAD_API_ROOT=staging
  endif
  RELEASE_NAME=juju-gui-$(RELEASE_VERSION)
-RELEASE_FILE=releases/$(RELEASE_NAME).tgz
+RELEASE_FILE=releases/$(RELEASE_NAME).xz
  RELEASE_SIGNATURE=releases/$(RELEASE_NAME).asc
  NPM_CACHE_VERSION=$(BZR_REVNO)
  NPM_CACHE_FILE=$(CURDIR)/releases/npm-cache-$(NPM_CACHE_VERSION).tgz

Then run ``BRANCH_IS_GOOD=1 make distfile``. Copy over the new release
to the charm's releases directory. If you compare the two file sizes in
the charm's releases directory, the difference should be dramatic.

$ ls -l
-rw-r--r-- 1 gary gary 5076088 Oct 14 09:44
juju-gui-0.10.1+build.1133.xz
-rw-r--r-- 1 gary gary 44840221 Oct 12 20:15 juju-gui-0.10.1.tgz

You can remove the tgz, and then run juju bootstrap and make deploy in
the charm root directory. Once it is deployed, you should be able to
log in and see the gui as usual, and you should be able to verify the
fact that you are using your custom release if you go to
/juju-ui/version.js, where you will see something like "var
jujuGuiVersionInfo=['unreleased', '1133'];".

Thank you!

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

https://codereview.appspot.com/14425057/

[Gary Poster (gary)]

Thank you very much, Brad.

https://codereview.appspot.com/14425057/

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2013-10-09 08:56:20 +0000
+++ hooks/utils.py	2013-10-14 14:26:39 +0000
@@ -163,7 +163,7 @@
         \d+\.\d+\.\d+  # Major, minor, and patch version numbers.
         (?:\+build\.\d+)?  # Optional bzr revno for development releases.
     )
-    \.tgz  # File extension.
+    \.(?:tgz|xz)  # File extension.
 """, re.VERBOSE)
 
 results_log = None
@@ -237,7 +237,7 @@
     for release in releases:
         for file_ in release.files:
             file_url = str(file_)
-            if file_url.endswith('.tgz'):
+            if file_url.endswith('.tgz') or file_url.endswith('.xz'):
                 filename = os.path.split(urlparse.urlsplit(file_url).path)[1]
                 return file_.file_link, filename
     raise ValueError('%r: file not found' % release_version)
@@ -706,7 +706,11 @@
     """
     log('Retrieving Juju GUI release.')
     if origin == 'url':
-        return download_release(version, 'url-release.tgz')
+        # "version" is a url.
+        _, _, extension = version.rpartition('.')
+        if extension not in ('tgz', 'xz'):
+            extension = 'xz'
+        return download_release(version, 'url-release.' + extension)
     if origin == 'local':
         path = get_release_file_path()
         log('Using a local release: {}'.format(path))

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2013-10-09 08:56:20 +0000
+++ tests/test_utils.py	2013-10-14 14:26:39 +0000
@@ -119,24 +119,26 @@
 @mock.patch('utils.log', mock.Mock())
 class TestFetchGuiRelease(unittest.TestCase):
 
-    release_path = '/my/release.tgz'
+    sources = tuple(
+        {'filename': 'release.' + extension,
+         'release_path': '/my/release.' + extension}
+        for extension in ('tgz', 'xz'))
 
     @contextmanager
-    def patch_launchpad(self, origin, version):
+    def patch_launchpad(self, origin, version, source):
         """Mock the functions used to download a release from Launchpad.
 
         Ensure all the functions are called correctly.
         """
-        url = 'http://launchpad.example.com/release.tgz/file'
-        filename = 'release.tgz'
+        url = 'http://launchpad.example.com/' + source['filename'] + '/file'
         patch_launchpad = mock.patch('utils.Launchpad')
         patch_get_launchpad_release = mock.patch(
             'utils.get_launchpad_release',
-            mock.Mock(return_value=(url, filename)),
+            mock.Mock(return_value=(url, source['filename'])),
         )
         patch_download_release = mock.patch(
             'utils.download_release',
-            mock.Mock(return_value=self.release_path),
+            mock.Mock(return_value=source['release_path']),
         )
         with patch_launchpad as mock_launchpad:
             with patch_get_launchpad_release as mock_get_launchpad_release:
@@ -146,57 +148,68 @@
         login.assert_called_once_with('Juju GUI charm', 'production')
         mock_get_launchpad_release.assert_called_once_with(
             login().projects['juju-gui'], origin, version)
-        mock_download_release.assert_called_once_with(url, filename)
+        mock_download_release.assert_called_once_with(url, source['filename'])
 
     @mock.patch('utils.download_release')
     def test_url(self, mock_download_release):
         # The release is retrieved from an URL.
-        mock_download_release.return_value = self.release_path
-        url = 'http://download.example.com/release.tgz'
-        path = fetch_gui_release('url', url)
-        self.assertEqual(self.release_path, path)
-        mock_download_release.assert_called_once_with(url, 'url-release.tgz')
+        for source in self.sources:
+            mock_download_release.return_value = source['release_path']
+            url = 'http://download.example.com/' + source['filename']
+            path = fetch_gui_release('url', url)
+            self.assertEqual(source['release_path'], path)
+            mock_download_release.assert_called_once_with(
+                url, 'url-' + source['filename'])
+            mock_download_release.reset_mock()
 
     @mock.patch('utils.get_release_file_path')
     def test_local(self, mock_get_release_file_path):
         # The last local release is requested.
-        mock_get_release_file_path.return_value = self.release_path
-        path = fetch_gui_release('local', None)
-        self.assertEqual(self.release_path, path)
-        mock_get_release_file_path.assert_called_once_with()
+        for source in self.sources:
+            mock_get_release_file_path.return_value = source['release_path']
+            path = fetch_gui_release('local', None)
+            self.assertEqual(source['release_path'], path)
+            mock_get_release_file_path.assert_called_once_with()
+            mock_get_release_file_path.reset_mock()
 
     @mock.patch('utils.get_release_file_path')
     def test_version_found(self, mock_get_release_file_path):
         # A release version is specified and found locally.
-        mock_get_release_file_path.return_value = self.release_path
-        path = fetch_gui_release('stable', '0.1.42')
-        self.assertEqual(self.release_path, path)
-        mock_get_release_file_path.assert_called_once_with('0.1.42')
+        for source in self.sources:
+            mock_get_release_file_path.return_value = source['release_path']
+            path = fetch_gui_release('stable', '0.1.42')
+            self.assertEqual(source['release_path'], path)
+            mock_get_release_file_path.assert_called_once_with('0.1.42')
+            mock_get_release_file_path.reset_mock()
 
     @mock.patch('utils.get_release_file_path')
     def test_version_not_found(self, mock_get_release_file_path):
-         # A release version is specified but not found locally.
-        mock_get_release_file_path.return_value = None
-        with self.patch_launchpad('stable', '0.1.42'):
-            path = fetch_gui_release('stable', '0.1.42')
-        self.assertEqual(self.release_path, path)
-        mock_get_release_file_path.assert_called_once_with('0.1.42')
+        # A release version is specified but not found locally.
+        for source in self.sources:
+            mock_get_release_file_path.return_value = None
+            with self.patch_launchpad('stable', '0.1.42', source):
+                path = fetch_gui_release('stable', '0.1.42')
+            self.assertEqual(source['release_path'], path)
+            mock_get_release_file_path.assert_called_once_with('0.1.42')
+            mock_get_release_file_path.reset_mock()
 
     @mock.patch('utils.get_release_file_path')
     def test_stable(self, mock_get_release_file_path):
         # The last stable release is requested.
-        with self.patch_launchpad('stable', None):
-            path = fetch_gui_release('stable', None)
-        self.assertEqual(self.release_path, path)
-        self.assertFalse(mock_get_release_file_path.called)
+        for source in self.sources:
+            with self.patch_launchpad('stable', None, source):
+                path = fetch_gui_release('stable', None)
+            self.assertEqual(source['release_path'], path)
+            self.assertFalse(mock_get_release_file_path.called)
 
     @mock.patch('utils.get_release_file_path')
     def test_trunk(self, mock_get_release_file_path):
         # The last development release is requested.
-        with self.patch_launchpad('trunk', None):
-            path = fetch_gui_release('trunk', None)
-        self.assertEqual(self.release_path, path)
-        self.assertFalse(mock_get_release_file_path.called)
+        for source in self.sources:
+            with self.patch_launchpad('trunk', None, source):
+                path = fetch_gui_release('trunk', None)
+            self.assertEqual(source['release_path'], path)
+            self.assertFalse(mock_get_release_file_path.called)
 
 
 class TestFirstPathInDir(unittest.TestCase):
@@ -330,6 +343,16 @@
             path = get_release_file_path()
         self.assert_path('juju-gui-2.0.1.tgz', path)
 
+    def test_xz(self):
+        # The last release is correctly retrieved for xz files too.
+        self.add('juju-gui-0.12.1.tgz')
+        self.add('juju-gui-1.2.3.tgz')
+        self.add('juju-gui-2.0.0+build.42.tgz')
+        self.add('juju-gui-2.0.1.xz')
+        with self.mock_releases_dir():
+            path = get_release_file_path()
+        self.assert_path('juju-gui-2.0.1.xz', path)
+
     def test_ordering(self):
         # Release versions are correctly ordered.
         self.add('juju-gui-0.12.1.tgz')
@@ -591,6 +614,24 @@
         self.assertEqual('http://example.com/0.1.0.tgz', url)
         self.assertEqual('0.1.0.tgz', name)
 
+    def test_xz_files_are_found(self):
+        project = AttrDict(
+            series=[
+                AttrDict(
+                    name='stable',
+                    releases=[
+                        AttrDict(
+                            version='0.1.0',
+                            files=[FileStub('http://example.com/0.1.0.xz')],
+                        ),
+                    ],
+                ),
+            ],
+        )
+        url, name = get_launchpad_release(project, 'stable', None)
+        self.assertEqual('http://example.com/0.1.0.xz', url)
+        self.assertEqual('0.1.0.xz', name)
+
 
 class TestGetZookeeperAddress(unittest.TestCase):