Merge lp:~joetalbott/snappy-proposed-image-builder/add_versions into lp:snappy-proposed-image-builder

[Joe Talbott]

Add rootfs, image, and device versions to output payload.

Add rootfs, image, and device versions to output payload.

[Paul Larson (pwlars)]

Couldn't you have just done something like:
(payload['image_version'], payload['rootfs_version'], payload['device_version']) = image_versions.split(':')

review: Needs Information

[Celso Providelo (cprov)]

Joe,

Thanks for adding this.

I have few comments inline, but nothing blocking, just a suggestion and a clarification request.

review: Approve

[lp:~joetalbott/snappy-proposed-image-builder/add_versions]

14. By Joe Talbott on 2015-06-02

Remove a few layers of indirection.

15. By Joe Talbott on 2015-06-02

We're not using the combined image name so remove the code.

[Paul Larson (pwlars)]

Sorry, I think I mistyped the last comment. I meant - why do you need _get_image_versions() at all? Couldn't you have just called get_latest_image_version() and then used .split(':') to get the pieces of it that you need for the other fields?

[lp:~joetalbott/snappy-proposed-image-builder/add_versions]

16. By Joe Talbott on 2015-06-02

Avoid having payload['xyz'] all over the place.

[Paul Larson (pwlars)]

review: Approve

=== modified file 'snappy_proposed_image_builder/worker.py'
--- snappy_proposed_image_builder/worker.py	2015-06-02 01:58:29 +0000
+++ snappy_proposed_image_builder/worker.py	2015-06-02 17:16:41 +0000
@@ -67,13 +67,18 @@
             )
             return MessageActions.Retry
 
-        image_name = get_latest_image_version(release, channel, device)
+        (image_version, rootfs_version, device_version) = _get_image_versions(
+            release, channel, device)
+
+        payload['image_version'] = image_version
+        payload['rootfs_version'] = rootfs_version
+        payload['device_version'] = device_version
 
         with tempfile.TemporaryDirectory() as tmpdir:
             logger.info("Beginning rootfs download.", extra=extra)
             try:
                 rootfs_path = download_rootfs(
-                    image_name.split(':')[0],
+                    image_version,
                     release,
                     channel,
                     device,
@@ -103,7 +108,7 @@
                 os.remove(rootfs_path)
                 try:
                     rootfs_path = download_rootfs(
-                        image_name.split(':')[0],
+                        image_version,
                         channel,
                         device,
                         tmpdir
@@ -129,7 +134,7 @@
             try:
                 logger.info("Building the image")
                 image_path = build_image(
-                    image_name.split(':')[0],
+                    image_version,
                     new_rootfs_path,
                     release,
                     channel,
@@ -432,24 +437,22 @@
         return latest_image_info
 
 
-def get_latest_image_version(release, channel, device):
-    """Returns latest image version in
-    `version:ubuntu-version:raw_device-version` format
-    """
+def _get_image_versions(release, channel, device):
     latest_image_info = _get_version_string_output(release, channel, device)
-    version = ''
-    ubuntu_version = ''
+    image_version = ''
+    rootfs_version = ''
     dev_version = ''
     for info in latest_image_info.decode('utf-8').split("\n"):
         if 'Description' in info:
             for l in info.split(','):
                 if 'version' in l:
-                    version = l.split('version=')[1]
+                    image_version = l.split('version=')[1]
                     continue
                 if 'ubuntu' in l:
-                    ubuntu_version = l.split('ubuntu=')[1]
+                    rootfs_version = l.split('ubuntu=')[1]
                     continue
                 if 'raw-device' in l:
                     dev_version = l.split('raw-device=')[1]
                     continue
-    return (':'.join([version, ubuntu_version, dev_version]))
+
+    return (image_version, rootfs_version, dev_version)