From a719a80a5e65998901fec8283fe13ac3ad53eeed Mon Sep 17 00:00:00 2001 From: Keshav Varma Date: Wed, 12 Aug 2015 18:00:17 -0700 Subject: [PATCH 1/4] Allow a list of servers to be passed in to the client --- chronos/__init__.py | 37 +++++++++++++++++++++++++++++-------- itests/itest_utils.py | 2 -- tests/test_chronos_init.py | 20 +++++++++++++++++--- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/chronos/__init__.py b/chronos/__init__.py index ec539f9..307fd9d 100755 --- a/chronos/__init__.py +++ b/chronos/__init__.py @@ -27,12 +27,21 @@ import logging +class ChronosException(Exception): + pass + + +class ChronosInvalidJobException(Exception): + pass + + class ChronosClient(object): _user = None _password = None - def __init__(self, hostname, proto="http", username=None, password=None, level='WARN'): - self.baseurl = "%s://%s" % (proto, hostname) + def __init__(self, servers, proto="http", username=None, password=None, level='WARN'): + server_list = servers if isinstance(servers, list) else [servers] + self.servers = ["%s://%s" % (proto, server) for server in server_list] if username and password: self._user = username self._password = password @@ -85,9 +94,21 @@ def _call(self, url, method="GET", body=None, headers={}): conn = httplib2.Http(disable_ssl_certificate_validation=True) if self._user and self._password: conn.add_credentials(self._user, self._password) - endpoint = "%s%s" % (self.baseurl, url) - self.logger.debug(endpoint) - return self._check(*conn.request(endpoint, method, body=body, headers=hdrs)) + + response = None + servers = list(self.servers) + while servers: + server = servers.pop(0) + endpoint = "%s%s" % (server, url) + self.logger.debug(endpoint) + try: + response = self._check(*conn.request(endpoint, method, body=body, headers=hdrs)) + self.logger.info('Got response from %s', endpoint) + return response + except Exception as e: + self.logger.error('Error while calling %s: %s', endpoint, e.message) + + raise ChronosException('No remaining Chronos servers to try') def _check(self, resp, content): status = resp.status @@ -112,7 +133,7 @@ def _check_fields(self, job): for k in ChronosJob.one_of: if k in job: return True - raise Exception("Job must include one of %s" % ChronosJob.one_of) + raise ChronosInvalidJobException("Job must include one of %s" % ChronosJob.one_of) class ChronosJob(object): @@ -127,5 +148,5 @@ class ChronosJob(object): one_of = ["schedule", "parents"] -def connect(hostname, proto="http", username=None, password=None): - return ChronosClient(hostname, proto="http", username=username, password=password) +def connect(servers, proto="http", username=None, password=None): + return ChronosClient(servers, proto=proto, username=username, password=password) diff --git a/itests/itest_utils.py b/itests/itest_utils.py index 5406c01..1ee54fa 100644 --- a/itests/itest_utils.py +++ b/itests/itest_utils.py @@ -2,8 +2,6 @@ from functools import wraps import os import signal -import sys -import threading import time import requests diff --git a/tests/test_chronos_init.py b/tests/test_chronos_init.py index 85d5113..3e6271a 100644 --- a/tests/test_chronos_init.py +++ b/tests/test_chronos_init.py @@ -1,12 +1,26 @@ import json - import mock import chronos +def test_connect_accepts_single_host(): + client = chronos.ChronosClient("localhost", proto="http") + assert client.servers == ['http://localhost'] + + +def test_connect_accepts_list_of_hosts(): + client = chronos.ChronosClient(["host1", "host2"], proto="http") + assert client.servers == ['http://host1', 'http://host2'] + + +def test_connect_accepts_proto(): + client = chronos.ChronosClient("localhost", proto="fake_proto") + assert client.servers == ['fake_proto://localhost'] + + def test_check_accepts_json(): - client = chronos.ChronosClient(hostname="localhost") + client = chronos.ChronosClient("localhost") fake_response = mock.Mock() fake_response.status = 200 fake_content = '{ "foo": "bar" }' @@ -15,7 +29,7 @@ def test_check_accepts_json(): def test_check_returns_raw_response_when_not_json(): - client = chronos.ChronosClient(hostname="localhost") + client = chronos.ChronosClient("localhost") fake_response = mock.Mock() fake_response.status = 401 fake_content = 'UNAUTHORIZED' From 6c4a0ca7d7e99bf0c42465a3eb4ea02c5c836775 Mon Sep 17 00:00:00 2001 From: Keshav Varma Date: Wed, 12 Aug 2015 18:45:34 -0700 Subject: [PATCH 2/4] Add tests for retrying against list of servers --- tests/test_chronos_init.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_chronos_init.py b/tests/test_chronos_init.py index 3e6271a..3f39b21 100644 --- a/tests/test_chronos_init.py +++ b/tests/test_chronos_init.py @@ -35,3 +35,14 @@ def test_check_returns_raw_response_when_not_json(): fake_content = 'UNAUTHORIZED' actual = client._check(fake_response, fake_content) assert actual == fake_content + + +def test_uses_server_list(): + client = chronos.ChronosClient(["host1", "host2", "host3"], proto="http") + good_request = (mock.Mock(status=204), '') + bad_request = (mock.Mock(status=500), '') + + conn_mock = mock.Mock(request=mock.Mock(side_effect=[bad_request, good_request, bad_request])) + with mock.patch('httplib2.Http', return_value=conn_mock): + client._call('/fake_url') + assert conn_mock.request.call_count == 2 From 5ee5d0f4e9711c011d852ca89ac2d716cfc73bf2 Mon Sep 17 00:00:00 2001 From: Robert Johnson Date: Fri, 21 Aug 2015 05:45:43 -0700 Subject: [PATCH 3/4] use more specific exceptions for failures --- chronos/__init__.py | 14 +++++++++++--- tests/test_chronos_init.py | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/chronos/__init__.py b/chronos/__init__.py index ec539f9..07b62bb 100755 --- a/chronos/__init__.py +++ b/chronos/__init__.py @@ -27,6 +27,14 @@ import logging +class ChronosAPIException(Exception): + pass + + +class MissingFieldException(Exception): + pass + + class ChronosClient(object): _user = None _password = None @@ -101,18 +109,18 @@ def _check(self, resp, content): payload = content if payload is None and status != 204: - raise Exception("HTTP Error %d occurred." % status) + raise ChronosAPIException("Request to Chronos API failed: status: %d, response: %s" % (status, content)) return payload def _check_fields(self, job): for k in ChronosJob.fields: if k not in job: - raise Exception("missing required field %s" % k) + raise MissingFieldException("missing required field %s" % k) for k in ChronosJob.one_of: if k in job: return True - raise Exception("Job must include one of %s" % ChronosJob.one_of) + raise MissingFieldException("Job must include one of %s" % ChronosJob.one_of) class ChronosJob(object): diff --git a/tests/test_chronos_init.py b/tests/test_chronos_init.py index 85d5113..8a82ce2 100644 --- a/tests/test_chronos_init.py +++ b/tests/test_chronos_init.py @@ -1,6 +1,8 @@ import json import mock +import pytest +import httplib2 import chronos @@ -21,3 +23,28 @@ def test_check_returns_raw_response_when_not_json(): fake_content = 'UNAUTHORIZED' actual = client._check(fake_response, fake_content) assert actual == fake_content + + +def test_api_error_throws_exception(): + client = chronos.ChronosClient(hostname="localhost") + mock_response = mock.Mock() + mock_response.status = 500 + mock_request = mock.Mock(return_value=(mock_response, None)) + with mock.patch.object(httplib2.Http, 'request', mock_request): + with pytest.raises(chronos.ChronosAPIException): + client.list() + + +def test_check_missing_fields(): + client = chronos.ChronosClient(hostname="localhost") + for field in chronos.ChronosJob.fields: + without_field = {x: 'foo' for x in filter(lambda y: y != field, chronos.ChronosJob.fields)} + with pytest.raises(chronos.MissingFieldException): + client._check_fields(without_field) + + +def test_check_one_of(): + client = chronos.ChronosClient(hostname="localhost") + job = {field: 'foo' for field in chronos.ChronosJob.fields} + with pytest.raises(chronos.MissingFieldException): + client._check_fields(job) From 4b8aa18ed67cabb8c217db20cfd7a735d9ee9926 Mon Sep 17 00:00:00 2001 From: Keshav Varma Date: Thu, 27 Aug 2015 10:07:36 -0700 Subject: [PATCH 4/4] Update readme with example of using multiple servers --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index bcc1f86..9cae469 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,8 @@ Simple python api for the chronos job scheduler ``` import chronos c = chronos.connect("chronos.mesos.server.com:8080") +# or specify multilple servers that will be tried in order +c = chronos.connect(["chronos1.mesos.server.com:8080", "chronos2.mesos.server.com:8080"]) # get list of scheduled jobs and their status as # [{ 'name': 'job1', ..}, { 'name': 'job2', ..}]