Merge lp:~mvo/ubuntu-system-image/json-output into lp:~registry/ubuntu-system-image/client

[Michael Vogt]

This branch implements --json-ouput as described in bug #1423622.

Note that I did not test this yet, I have a bit of difficulties with
that. When I run "tox" on trunk I get all sorts of errors on my vivid
box that look unrelated. I also could not find a debian/ packaging
branch to test for real. Pointers appreciated :) I'm happy to do more
serious testing once I figured the testsuite and the debian/ dir out.

[Barry Warsaw (barry)]

I'd love to know what problems you had with tox. Be sure you have all the build dependencies though: `sudo apt-get build-dep system-image` should install them all (except for the new pycurl dependencies w/3.0, but I bet you already have those)

Normally, the packaging branch is here: bzr+ssh://bazaar.launchpad.net/~ubuntu-managed-branches/ubuntu-system-image/system-image/

But I am working on an mp for that to build si 3.0: bzr+ssh://bazaar.launchpad.net/~barry/ubuntu-system-image/citrain30beta/

Yeah, it's kind of schizophrenic and we should probably merge the debian/ into trunk, but hysterical raisins are so delicious!

[Barry Warsaw (barry)]

It's a nice simple branch (without the testing :) but I wonder if we'd ever want other formats of stdout progress, and whether we should extend the existing callback mechanism to implement this.

Right now, DownloadManagerBase takes a single callback function. What if instead we could register multiple callback functions and then _do_callback() would call each in turn? It would have to make sure that errors in one callback don't affect the others of course. But then it would just be a matter of implementing the JSON-to-stdout callback and adding it to the normal downloader callback stack.

The other thought I had was about the cli u/i. Maybe `--progress=json` would be translated into adding the JSON callback to the downloader stack. Or maybe `--progress=jsonv1` just to future proof us against possible different JSON contents?

main.py already adds a single callback function (e.g. differently depending on whether verbosity == 1 or not, though that should probably be verbosity >= 1). If state.downloader.callback were a list, it could just append to it instead of assigning to it. We could even allow for `--progress=dots` and `--progress=debuglog` to handle the current cases.

[Barry Warsaw (barry)]

Oh yeah, and don't forget the PPA! https://launchpad.net/~barry/+archive/ubuntu/systemimage

You can always grab the packages from there and test those (I'll be sure to upload a new version once this feature lands).

[Barry Warsaw (barry)]

https://code.launchpad.net/~barry/ubuntu-system-image/lp1423622/+merge/251819

Unmerged revisions

318. By Michael Vogt on 2015-02-27

minimal implementation of --json-output

=== modified file 'systemimage/config.py'
--- systemimage/config.py	2015-02-13 21:44:50 +0000
+++ systemimage/config.py	2015-02-27 09:33:32 +0000
@@ -72,6 +72,8 @@
         # This is used to plumb command line arguments from the main() to
         # other parts of the system.
         self.skip_gpg_verification = False
+        # json output when using the cli
+        self.json_output = False
         # Cache.
         self._device = None
         self._build_number = None

=== modified file 'systemimage/download.py'
--- systemimage/download.py	2015-02-13 21:44:50 +0000
+++ systemimage/download.py	2015-02-27 09:33:32 +0000
@@ -36,7 +36,6 @@
 except ImportError:                                 # pragma: no cover
     pycurl = None
 
-
 log = logging.getLogger('systemimage')
 
 
@@ -133,6 +132,12 @@
                 self.callback(self.received, self.total)
             except:
                 log.exception('Exception in progress callback')
+        from systemimage.config import config
+        if config.json_output:
+            print(json.dumps({
+                "type": "progress",
+                "now": self.received,
+                "total": self.total}))
 
     def cancel(self):
         """Cancel any current downloads."""

=== modified file 'systemimage/main.py'
--- systemimage/main.py	2015-02-09 18:46:24 +0000
+++ systemimage/main.py	2015-02-27 09:33:32 +0000
@@ -141,6 +141,9 @@
                         help="""Delete the key and its value.  It is a no-op
                                 if the key does not exist.  Multiple
                                 --del arguments can be given.""")
+    parser.add_argument('--json-output',
+                        default=False, action='store_true',
+                        help="""Show json progress/error output on stdout.""")
     # Hidden system-image-cli only feature for testing purposes.  LP: #1333414
     parser.add_argument('--skip-gpg-verification',
                         default=False, action='store_true',
@@ -160,6 +163,9 @@
 Your upgrades are INSECURE.""", file=sys.stderr)
         config.skip_gpg_verification = True
 
+    # store the json_output choice
+    config.json_output = args.json_output
+        
     # Perform factory and production resets.
     if args.factory_reset:
         factory_reset()
@@ -347,8 +353,10 @@
                 list(state)
         except KeyboardInterrupt:                   # pragma: no cover
             return 0
-        except Exception:
+        except Exception as e:
             log.exception('system-image-cli exception')
+            if config.json_output:
+                print(json.dumps({"type": "error", "msg": str(e)}))
             return 1
         else:
             return 0