From 7e904bc38a32eee6dbb287937f6957392fe1a635 Mon Sep 17 00:00:00 2001
From: Ken Sipe <kensipe@gmail.com>
Date: Fri, 2 Dec 2016 16:58:44 -0600
Subject: [PATCH] metronome: better reporting of json processing failures
 (#835)

---
 cli/dcoscli/job/main.py                 | 30 ++++++-------------------
 cli/tests/integrations/test_marathon.py | 15 ++++++++-----
 dcos/errors.py                          | 20 +++++++++++++++++
 dcos/http.py                            |  5 ++++-
 4 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/cli/dcoscli/job/main.py b/cli/dcoscli/job/main.py
index 23e821a..3c29fce 100644
--- a/cli/dcoscli/job/main.py
+++ b/cli/dcoscli/job/main.py
@@ -487,17 +487,7 @@ def _add_schedules(job_id, schedules_json):
 
     schedule = parse_schedule_json(schedules_json)
     client = metronome.create_client()
-    try:
-        client.add_schedule(job_id, schedule)
-    except DCOSHTTPException as e:
-        if e.response.status_code == 404:
-            emitter.publish("Job ID: '{}' does NOT exist.".format(job_id))
-        elif e.response.status_code == 409:
-            emitter.publish("Schedule already exists.")
-        else:
-            raise DCOSException(e)
-    except DCOSException as e:
-        raise DCOSException(e)
+    client.add_schedule(job_id, schedule)
 
     return 0
 
@@ -579,6 +569,9 @@ def _add_job(job_file):
     if 'id' not in full_json:
         raise DCOSException("Jobs JSON requires an ID.")
 
+    if 'disk' not in full_json['run']:
+        full_json['run']['disk'] = 0
+
     job_id = full_json['id']
     schedules = None
 
@@ -586,19 +579,10 @@ def _add_job(job_file):
         schedules = full_json['schedules']
         del full_json['schedules']
 
-    # iterate and post each schedule
-    job_added = False
-    try:
-        client = metronome.create_client()
-        client.add_job(full_json)
-        job_added = True
-    except DCOSHTTPException as e:
-        if e.response.status_code == 409:
-            emitter.publish("Job ID: '{}' already exists".format(job_id))
-        else:
-            raise DCOSException(e)
+    client = metronome.create_client()
+    client.add_job(full_json)
 
-    if schedules is not None and job_added is True:
+    if schedules is not None:
         return _add_schedules(job_id, schedules)
 
     return 0
diff --git a/cli/tests/integrations/test_marathon.py b/cli/tests/integrations/test_marathon.py
index 82eb8c5..ae0644a 100644
--- a/cli/tests/integrations/test_marathon.py
+++ b/cli/tests/integrations/test_marathon.py
@@ -187,13 +187,16 @@ def test_show_bad_app_version():
             'zero-instance-app',
             'tests/data/marathon/apps/update_zero_instance_sleep.json')
 
-        stderr = (b'Error: Invalid format: "20:39:32.972Z" is malformed at '
-                  b'":39:32.972Z"\n')
-        assert_command(
+        returncode, stdout, stderr = exec_command(
             ['dcos', 'marathon', 'app', 'show', '--app-version=20:39:32.972Z',
-             'zero-instance-app'],
-            returncode=1,
-            stderr=stderr)
+             'zero-instance-app'])
+        assert returncode == 1
+        assert stdout == b''
+        assert stderr.startswith(b'Error while fetching')
+        pattern = (b"""{"message":"Invalid format: """
+                   b"""\\"20:39:32.972Z\\" is malformed"""
+                   b""" at \\":39:32.972Z\\""}\n""")
+        assert stderr.endswith(pattern)
 
 
 def test_show_bad_relative_app_version():
diff --git a/dcos/errors.py b/dcos/errors.py
index 577b08d..6068891 100644
--- a/dcos/errors.py
+++ b/dcos/errors.py
@@ -24,6 +24,26 @@ class DCOSHTTPException(DCOSException):
             self.response.reason)
 
 
+class DCOSUnprocessableException(DCOSException):
+    """ A wrapper around Response objects for HTTP 422
+    error codes, Unprocessable JSON Entities.
+
+    :param response: requests Response object
+    :type response: Response
+    """
+    def __init__(self, response):
+        self.response = response
+
+    def status(self):
+        return self.response.status_code
+
+    def __str__(self):
+        return 'Error while fetching [{0}]: HTTP {1}: {2}'.format(
+            self.response.request.url,
+            self.response.status_code,
+            self.response.text)
+
+
 class DCOSAuthenticationException(DCOSHTTPException):
     """A wrapper around Response objects for HTTP Authentication errors (401).
 
diff --git a/dcos/http.py b/dcos/http.py
index 52d9d80..d2c918d 100644
--- a/dcos/http.py
+++ b/dcos/http.py
@@ -10,7 +10,8 @@ from six.moves.urllib.parse import urlparse
 from dcos import config, util
 from dcos.errors import (DCOSAuthenticationException,
                          DCOSAuthorizationException, DCOSBadRequest,
-                         DCOSException, DCOSHTTPException)
+                         DCOSException, DCOSHTTPException,
+                         DCOSUnprocessableException)
 
 logger = util.get_logger(__name__)
 lock = threading.Lock()
@@ -235,6 +236,8 @@ def request(method,
             raise DCOSException(msg)
         else:
             raise DCOSAuthenticationException(response)
+    elif response.status_code == 422:
+        raise DCOSUnprocessableException(response)
     elif response.status_code == 403:
         raise DCOSAuthorizationException(response)
     elif response.status_code == 400: