Merge pull request #108 from mesosphere/messaging
consistent marathon error messaging
This commit is contained in:
@@ -606,6 +606,35 @@ def test_show_task():
|
||||
_remove_app('zero-instance-app')
|
||||
|
||||
|
||||
def test_bad_configuration():
|
||||
returncode, port, stderr = exec_command(
|
||||
['dcos', 'config', 'show', 'marathon.port'])
|
||||
|
||||
assert returncode == 0
|
||||
assert stderr == b''
|
||||
|
||||
returncode, stdout, stderr = exec_command(
|
||||
['dcos', 'config', 'set', 'marathon.port', str(int(port) + 1)])
|
||||
assert returncode == 0
|
||||
assert stdout == b''
|
||||
assert stderr == b''
|
||||
|
||||
returncode, stdout, stderr = exec_command(
|
||||
['dcos', 'marathon', 'app', 'list'])
|
||||
|
||||
expected_message = b"Error: Marathon likely misconfigured. " + \
|
||||
b"Please check your marathon port and host settings. "
|
||||
|
||||
assert stderr.startswith(expected_message)
|
||||
|
||||
returncode, stdout, stderr = exec_command(
|
||||
['dcos', 'config', 'set', 'marathon.port', port])
|
||||
|
||||
assert returncode == 0
|
||||
assert stdout == b''
|
||||
assert stderr == b''
|
||||
|
||||
|
||||
def _list_apps(app_id=None):
|
||||
returncode, stdout, stderr = exec_command(
|
||||
['dcos', 'marathon', 'app', 'list'])
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import requests
|
||||
from dcos.api import util
|
||||
from dcos.api.errors import DefaultError
|
||||
from dcos.api.errors import DefaultError, Error
|
||||
|
||||
logger = util.get_logger(__name__)
|
||||
|
||||
@@ -17,21 +17,24 @@ def _default_is_success(status_code):
|
||||
return status_code >= 200 and status_code < 300
|
||||
|
||||
|
||||
def _default_response_to_error(response):
|
||||
def _default_to_error(response):
|
||||
"""
|
||||
:param response: HTTP resonse object
|
||||
:type response: requests.Response
|
||||
:param response: HTTP response object or Error
|
||||
:type response: requests.Response or Error
|
||||
:returns: the error embedded in the response JSON
|
||||
:rtype: Error
|
||||
"""
|
||||
|
||||
if isinstance(response, Error):
|
||||
return response
|
||||
|
||||
return DefaultError('{}: {}'.format(response.status_code, response.text))
|
||||
|
||||
|
||||
def request(method,
|
||||
url,
|
||||
is_success=_default_is_success,
|
||||
response_to_error=_default_response_to_error,
|
||||
to_error=_default_to_error,
|
||||
**kwargs):
|
||||
"""Sends an HTTP request.
|
||||
|
||||
@@ -41,8 +44,8 @@ def request(method,
|
||||
:type url: str
|
||||
:param is_success: Defines successful status codes for the request
|
||||
:type is_success: Function from int to bool
|
||||
:param response_to_error: Builds an Error from an unsuccessful response
|
||||
:type response_to_error: Function from requests.Response to Error
|
||||
:param to_error: Builds an Error from an unsuccessful response or Error
|
||||
:type to_error: Function from requests.Response or Error to Error
|
||||
:param kwargs: Additional arguments to requests.request
|
||||
(see http://docs.python-requests.org/en/latest/api/#requests.request)
|
||||
:type kwargs: dict
|
||||
@@ -66,7 +69,7 @@ def request(method,
|
||||
request.headers)
|
||||
|
||||
with requests.Session() as session:
|
||||
response = session.send(request.prepare())
|
||||
response = session.send(request.prepare(), timeout=3.0)
|
||||
|
||||
logger.info('Received HTTP response [%r]: %r',
|
||||
response.status_code,
|
||||
@@ -75,13 +78,13 @@ def request(method,
|
||||
if is_success(response.status_code):
|
||||
return (response, None)
|
||||
else:
|
||||
return (None, response_to_error(response))
|
||||
return (None, to_error(response))
|
||||
|
||||
except Exception as ex:
|
||||
return (None, DefaultError(str(ex)))
|
||||
return (None, to_error(DefaultError(str(ex))))
|
||||
|
||||
|
||||
def head(url, **kwargs):
|
||||
def head(url, to_error=_default_to_error, **kwargs):
|
||||
"""Sends a HEAD request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -95,7 +98,7 @@ def head(url, **kwargs):
|
||||
return request('head', url, **kwargs)
|
||||
|
||||
|
||||
def get(url, **kwargs):
|
||||
def get(url, to_error=_default_to_error, **kwargs):
|
||||
"""Sends a GET request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -106,10 +109,10 @@ def get(url, **kwargs):
|
||||
:rtype: (Response, Error)
|
||||
"""
|
||||
|
||||
return request('get', url, **kwargs)
|
||||
return request('get', url, to_error=to_error, **kwargs)
|
||||
|
||||
|
||||
def post(url, data=None, json=None, **kwargs):
|
||||
def post(url, to_error=_default_to_error, data=None, json=None, **kwargs):
|
||||
"""Sends a POST request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -124,10 +127,11 @@ def post(url, data=None, json=None, **kwargs):
|
||||
:rtype: (Response, Error)
|
||||
"""
|
||||
|
||||
return request('post', url, data=data, json=json, **kwargs)
|
||||
return request('post',
|
||||
url, to_error=to_error, data=data, json=json, **kwargs)
|
||||
|
||||
|
||||
def put(url, data=None, **kwargs):
|
||||
def put(url, to_error=_default_to_error, data=None, **kwargs):
|
||||
"""Sends a PUT request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -140,10 +144,10 @@ def put(url, data=None, **kwargs):
|
||||
:rtype: (Response, Error)
|
||||
"""
|
||||
|
||||
return request('put', url, data=data, **kwargs)
|
||||
return request('put', url, to_error=to_error, data=data, **kwargs)
|
||||
|
||||
|
||||
def patch(url, data=None, **kwargs):
|
||||
def patch(url, to_error=_default_to_error, data=None, **kwargs):
|
||||
"""Sends a PATCH request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -156,10 +160,10 @@ def patch(url, data=None, **kwargs):
|
||||
:rtype: (Response, Error)
|
||||
"""
|
||||
|
||||
return request('patch', url, data=data, **kwargs)
|
||||
return request('patch', url, to_error=to_error, data=data, **kwargs)
|
||||
|
||||
|
||||
def delete(url, **kwargs):
|
||||
def delete(url, to_error=_default_to_error, **kwargs):
|
||||
"""Sends a DELETE request.
|
||||
|
||||
:param url: URL for the new Request object
|
||||
@@ -170,4 +174,4 @@ def delete(url, **kwargs):
|
||||
:rtype: (Response, Error)
|
||||
"""
|
||||
|
||||
return request('delete', url, **kwargs)
|
||||
return request('delete', url, to_error=to_error, **kwargs)
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import json
|
||||
|
||||
from dcos.api import http, util
|
||||
from dcos.api.errors import DefaultError
|
||||
from dcos.api.errors import DefaultError, Error
|
||||
|
||||
try:
|
||||
from urllib import quote
|
||||
@@ -22,14 +22,17 @@ def create_client(config):
|
||||
return Client(config['marathon.host'], config['marathon.port'])
|
||||
|
||||
|
||||
def _response_to_error(response):
|
||||
def _to_error(response):
|
||||
"""
|
||||
:param response: HTTP resonse object
|
||||
:type response: requests.Response
|
||||
:param response: HTTP response object or Error
|
||||
:type response: requests.Response or Error
|
||||
:returns: the error embedded in the response JSON
|
||||
:rtype: Error
|
||||
"""
|
||||
|
||||
if isinstance(response, Error):
|
||||
return DefaultMarathonError(response.error())
|
||||
|
||||
message = response.json().get('message')
|
||||
if message is None:
|
||||
errs = response.json().get('errors')
|
||||
@@ -37,10 +40,10 @@ def _response_to_error(response):
|
||||
logger.error(
|
||||
'Marathon server did not return a message: %s',
|
||||
response.json())
|
||||
return DefaultError('Unknown error from Marathon')
|
||||
return DefaultMarathonError()
|
||||
|
||||
msg = '\n'.join(error['error'] for error in errs)
|
||||
return DefaultError('Error(s): {}'.format(msg))
|
||||
return DefaultMarathonError(msg)
|
||||
|
||||
return DefaultError('Error: {}'.format(response.json()['message']))
|
||||
|
||||
@@ -92,7 +95,7 @@ class Client(object):
|
||||
url = self._create_url(
|
||||
'v2/apps{}/versions/{}'.format(app_id, version))
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -127,7 +130,7 @@ class Client(object):
|
||||
|
||||
url = self._create_url('v2/apps{}/versions'.format(app_id))
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -146,7 +149,7 @@ class Client(object):
|
||||
|
||||
url = self._create_url('v2/apps')
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -173,7 +176,7 @@ class Client(object):
|
||||
|
||||
response, error = http.post(url,
|
||||
json=app_json,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -205,7 +208,7 @@ class Client(object):
|
||||
response, error = http.put(url,
|
||||
params=params,
|
||||
json=payload,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -237,7 +240,7 @@ class Client(object):
|
||||
response, error = http.put(url,
|
||||
params=params,
|
||||
json={'instances': int(instances)},
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -280,7 +283,7 @@ class Client(object):
|
||||
|
||||
response, error = http.delete(url,
|
||||
params=params,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return error
|
||||
@@ -309,7 +312,7 @@ class Client(object):
|
||||
|
||||
response, error = http.post(url,
|
||||
params=params,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -328,7 +331,7 @@ class Client(object):
|
||||
url = self._create_url('v2/deployments')
|
||||
|
||||
response, error = http.get(url,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -351,7 +354,7 @@ class Client(object):
|
||||
|
||||
url = self._create_url('v2/deployments')
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
@@ -391,7 +394,7 @@ class Client(object):
|
||||
response, error = http.delete(
|
||||
url,
|
||||
params=params,
|
||||
response_to_error=_response_to_error)
|
||||
to_error=_to_error)
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
|
||||
@@ -434,7 +437,7 @@ class Client(object):
|
||||
|
||||
url = self._create_url('v2/tasks')
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
|
||||
@@ -460,7 +463,7 @@ class Client(object):
|
||||
|
||||
url = self._create_url('v2/tasks')
|
||||
|
||||
response, error = http.get(url, response_to_error=_response_to_error)
|
||||
response, error = http.get(url, to_error=_to_error)
|
||||
if error is not None:
|
||||
return (None, error)
|
||||
|
||||
@@ -481,3 +484,16 @@ class Client(object):
|
||||
"""
|
||||
|
||||
return quote('/' + app_id.strip('/'))
|
||||
|
||||
|
||||
class DefaultMarathonError(DefaultError):
|
||||
"""Construct a basic Error class for Marathon
|
||||
|
||||
:param message: String to use for additional messaging
|
||||
:type message: str
|
||||
"""
|
||||
|
||||
def __init__(self, message=""):
|
||||
self._message = "Error: Marathon likely misconfigured. " + \
|
||||
"Please check your marathon port and host settings. " + \
|
||||
message
|
||||
|
||||
Reference in New Issue
Block a user