aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject incubator-aurora git commit: Catch only known Exception types in the client.
Date Wed, 11 Mar 2015 00:39:51 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 18cd5330e -> 7a6feee3f


Catch only known Exception types in the client.

Bugs closed: AURORA-1176

Reviewed at https://reviews.apache.org/r/31869/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/7a6feee3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/7a6feee3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/7a6feee3

Branch: refs/heads/master
Commit: 7a6feee3fdc20f35e0b30e8b4937a555b80b4433
Parents: 18cd533
Author: Bill Farner <wfarner@apache.org>
Authored: Tue Mar 10 17:38:42 2015 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Tue Mar 10 17:38:42 2015 -0700

----------------------------------------------------------------------
 .../python/apache/aurora/client/api/__init__.py |  7 +++--
 .../python/apache/aurora/client/cli/__init__.py | 25 ++++++-----------
 .../python/apache/aurora/client/cli/jobs.py     | 12 +++++----
 .../aurora/client/cli/test_api_from_cli.py      | 10 +++----
 .../apache/aurora/client/cli/test_create.py     | 28 ++++++++------------
 .../apache/aurora/client/cli/test_kill.py       | 15 +++--------
 .../apache/aurora/client/cli/test_supdate.py    |  3 ++-
 .../python/apache/aurora/client/cli/util.py     | 22 +++++----------
 8 files changed, 46 insertions(+), 76 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/main/python/apache/aurora/client/api/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/api/__init__.py b/src/main/python/apache/aurora/client/api/__init__.py
index 194629f..4025781 100644
--- a/src/main/python/apache/aurora/client/api/__init__.py
+++ b/src/main/python/apache/aurora/client/api/__init__.py
@@ -44,7 +44,6 @@ class AuroraClientAPI(object):
   """This class provides the API to talk to the twitter scheduler"""
 
   class Error(Exception): pass
-  class TypeError(Error, TypeError): pass
   class ClusterMismatch(Error, ValueError): pass
   class ThriftInternalError(Error): pass
   class UpdateConfigError(Error): pass
@@ -329,12 +328,12 @@ class AuroraClientAPI(object):
 
   def _assert_valid_lock(self, lock):
     if not isinstance(lock, Lock):
-      raise self.TypeError('Invalid lock %r: expected %s but got %s'
-                           % (lock, AuroraJobKey.__name__, lock.__class__.__name__))
+      raise TypeError('Invalid lock %r: expected %s but got %s'
+          % (lock, AuroraJobKey.__name__, lock.__class__.__name__))
 
   def _assert_valid_job_key(self, job_key):
     if not isinstance(job_key, AuroraJobKey):
-      raise self.TypeError('Invalid job_key %r: expected %s but got %s'
+      raise TypeError('Invalid job_key %r: expected %s but got %s'
           % (job_key, AuroraJobKey.__name__, job_key.__class__.__name__))
     if job_key.cluster != self.cluster.name:
       raise self.ClusterMismatch('job %s does not belong to cluster %s' % (job_key,

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/main/python/apache/aurora/client/cli/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/__init__.py b/src/main/python/apache/aurora/client/cli/__init__.py
index 4d9ef09..6a0c129 100644
--- a/src/main/python/apache/aurora/client/cli/__init__.py
+++ b/src/main/python/apache/aurora/client/cli/__init__.py
@@ -39,6 +39,8 @@ from abc import abstractmethod, abstractproperty
 import pkg_resources
 from twitter.common.lang import AbstractClass, Compatibility
 
+from apache.aurora.client.api import AuroraClientAPI
+
 from .command_hooks import GlobalCommandHookRegistry
 from .options import CommandOption
 
@@ -191,14 +193,6 @@ class CommandLine(AbstractClass):
   def name(self):
     """Returns the name of this command-line tool"""
 
-  def print_out(self, s, indent=0):
-    indent_str = " " * indent
-    print("%s%s" % (indent_str, s))
-
-  def print_err(self, s, indent=0):
-    indent_str = " " * indent
-    print("%s%s" % (indent_str, s), file=sys.stderr)
-
   def __init__(self):
     self.nouns = None
     self.parser = None
@@ -277,7 +271,7 @@ class CommandLine(AbstractClass):
       for plugin in self.plugins:
         plugin.before_execution(context)
     except ConfigurationPlugin.Error as e:
-      print("Error in configuration plugin before execution: %s" % e.msg, file=sys.stderr)
+      context.print_err("Error in configuration plugin before execution: %s" % e.msg)
       return e.code
     plugin_result = GlobalCommandHookRegistry.run_pre_hooks(context, context.options.noun,
         context.options.verb)
@@ -322,12 +316,12 @@ class CommandLine(AbstractClass):
     except Context.CommandErrorLogged as c:
       return c.code
     except Context.CommandError as c:
-      self.print_err("Error executing command: %s" % c.msg)
+      context.print_err("Error executing command: %s" % c.msg)
       return c.code
-    except Exception:
-      # TODO(wfarner): Remove this block - it hides exceptions and complicates testing.
-      print("Fatal error running command:", file=sys.stderr)
-      print(traceback.format_exc(), file=sys.stderr)
+    except AuroraClientAPI.Error as e:
+      # TODO(wfarner): Generalize this error type in the contract of noun and verb implementations.
+      context.print_err("Fatal error running command: %s" % e.message)
+      context.print_err(traceback.format_exc())
       return EXIT_UNKNOWN_ERROR
 
   def execute(self, args):
@@ -336,9 +330,6 @@ class CommandLine(AbstractClass):
     except KeyboardInterrupt:
       logging.error("Command interrupted by user")
       return EXIT_INTERRUPTED
-    except Exception as e:
-      logging.error("Unknown error: %s" % e)
-      return EXIT_UNKNOWN_ERROR
 
 
 class Noun(AuroraCommand):

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/main/python/apache/aurora/client/cli/jobs.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/jobs.py b/src/main/python/apache/aurora/client/cli/jobs.py
index 286b218..d6633d9 100644
--- a/src/main/python/apache/aurora/client/cli/jobs.py
+++ b/src/main/python/apache/aurora/client/cli/jobs.py
@@ -655,13 +655,15 @@ class StatusCommand(Verb):
     else:
       return "".join(result)
 
-  def _render_partial_jobkey(self, jobkey):
+  @classmethod
+  def _render_partial_jobkey(cls, jobkey):
     return "%s/%s/%s/%s" % jobkey
 
-  def _print_jobs_not_found(self, context):
+  @classmethod
+  def _print_jobs_not_found(cls, context):
     if context.options.write_json:
       context.print_out(json.dumps(
-        {"jobspec": self._render_partial_jobkey(context.options.jobspec),
+        {"jobspec": cls._render_partial_jobkey(context.options.jobspec),
          "error": "No matching jobs found"},
         separators=[",", ":"],
         sort_keys=False))
@@ -670,13 +672,13 @@ class StatusCommand(Verb):
       return EXIT_OK
     else:
       context.print_err("Found no jobs matching %s" %
-          self._render_partial_jobkey(context.options.jobspec))
+          cls._render_partial_jobkey(context.options.jobspec))
       return EXIT_INVALID_PARAMETER
 
   def execute(self, context):
     jobs = context.get_jobs_matching_key(context.options.jobspec)
     if not jobs:
-      return self._print_jobs_not_found()
+      return self._print_jobs_not_found(context)
 
     result = self.get_status_for_jobs(jobs, context)
     if result is not None:

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/test/python/apache/aurora/client/cli/test_api_from_cli.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_api_from_cli.py b/src/test/python/apache/aurora/client/cli/test_api_from_cli.py
index b855c3c..ba59957 100644
--- a/src/test/python/apache/aurora/client/cli/test_api_from_cli.py
+++ b/src/test/python/apache/aurora/client/cli/test_api_from_cli.py
@@ -14,7 +14,7 @@
 
 import contextlib
 
-from mock import create_autospec, patch
+from mock import call, create_autospec, patch
 
 from apache.aurora.client.api.scheduler_client import SchedulerClient
 from apache.aurora.client.cli import EXIT_UNKNOWN_ERROR
@@ -76,8 +76,8 @@ class TestApiFromCLI(AuroraClientCommandTest):
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS)):
       cmd = AuroraCommandLine()
       cmd.execute(['job', 'status', 'west/bozo/test/hello'])
-      mock_thrift_client.getTasksWithoutConfigs.assert_called_with(
-          TaskQuery(jobKeys=[JobKey(role='bozo', environment='test', name='hello')]))
+      assert mock_thrift_client.getTasksWithoutConfigs.mock_calls == [
+          call(TaskQuery(jobKeys=[JobKey(role='bozo', environment='test', name='hello')]))]
 
   def test_status_api_failure(self):
     mock_scheduler_client = create_autospec(spec=SchedulerClient, instance=True)
@@ -96,5 +96,5 @@ class TestApiFromCLI(AuroraClientCommandTest):
       # exception, which results in the command failing with an error code.
       result = cmd.execute(['job', 'status', 'west/bozo/test/hello'])
       assert result == EXIT_UNKNOWN_ERROR
-      mock_thrift_client.getTasksWithoutConfigs.assert_called_with(
-        TaskQuery(jobKeys=[JobKey(role='bozo', environment='test', name='hello')]))
+      assert mock_thrift_client.getTasksWithoutConfigs.mock_calls == [
+        call(TaskQuery(jobKeys=[JobKey(role='bozo', environment='test', name='hello')]))]

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/test/python/apache/aurora/client/cli/test_create.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_create.py b/src/test/python/apache/aurora/client/cli/test_create.py
index 459d157..57970c4 100644
--- a/src/test/python/apache/aurora/client/cli/test_create.py
+++ b/src/test/python/apache/aurora/client/cli/test_create.py
@@ -23,20 +23,14 @@ from apache.aurora.client.cli import (
     EXIT_COMMAND_FAILURE,
     EXIT_INTERRUPTED,
     EXIT_INVALID_CONFIGURATION,
-    EXIT_OK,
-    EXIT_UNKNOWN_ERROR
+    EXIT_OK
 )
 from apache.aurora.client.cli.client import AuroraCommandLine
 from apache.aurora.client.cli.jobs import CreateJobCommand
 from apache.aurora.common.aurora_job_key import AuroraJobKey
 from apache.aurora.config import AuroraConfig
 
-from .util import (
-    AuroraClientCommandTest,
-    FakeAuroraCommandContext,
-    FakeAuroraCommandLine,
-    mock_verb_options
-)
+from .util import AuroraClientCommandTest, FakeAuroraCommandContext, mock_verb_options
 
 from gen.apache.aurora.api.ttypes import (
     JobKey,
@@ -266,18 +260,15 @@ class TestClientCreateCommand(AuroraClientCommandTest):
     with contextlib.nested(
         patch('time.sleep'),
         patch('time.time', return_value=23),
-        patch('apache.aurora.client.cli.jobs.Job.create_context',
-            side_effect=Exception("Argh"))):
+        patch('apache.aurora.client.cli.jobs.Job.create_context', side_effect=UnknownException())):
       api = mock_context.get_api('west')
       api.create_job.return_value = self.get_createjob_response()
       with temporary_file() as fp:
         fp.write(self.get_valid_config())
         fp.flush()
         cmd = AuroraCommandLine()
-        result = cmd.execute(['job', 'create', '--wait-until=RUNNING', 'west/bozo/test/hello',
-            fp.name])
-        assert result == EXIT_UNKNOWN_ERROR
-        assert api.create_job.call_count == 0
+        with pytest.raises(UnknownException):
+          cmd.execute(['job', 'create', '--wait-until=RUNNING', 'west/bozo/test/hello', fp.name])
 
   def test_simple_successful_create_job_output(self):
     """Run a test of the "create" command against a mocked-out API:
@@ -394,20 +385,23 @@ class TestClientCreateCommand(AuroraClientCommandTest):
     Verifies that the creation command sends the right API RPCs, and performs the correct
     tests on the result."""
 
+    mock_context = FakeAuroraCommandContext()
     with contextlib.nested(
-        patch('threading._Event.wait')):
+        patch('threading._Event.wait'),
+        patch('apache.aurora.client.cli.jobs.Job.create_context', return_value=mock_context)):
 
       # This is the real test: invoke create as if it had been called by the command line.
       with temporary_file() as fp:
         fp.write(self.get_unbound_test_config())
         fp.flush()
-        cmd = FakeAuroraCommandLine()
+        cmd = AuroraCommandLine()
         result = cmd.execute(['job', 'create', '--wait-until=RUNNING',
             '--bind', 'cluster_binding=west',
            'west/bozo/test/hello',
             fp.name])
         assert result == EXIT_INVALID_CONFIGURATION
-        assert cmd.get_err() == [
+      assert mock_context.get_out() == []
+      assert mock_context.get_err() == [
             "Error executing command: Error loading configuration: "
             "TypeCheck(FAILED): MesosJob[update_config] failed: "
             "UpdateConfig[batch_size] failed: u'{{TEST_BATCH}}' not an integer"]

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/test/python/apache/aurora/client/cli/test_kill.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_kill.py b/src/test/python/apache/aurora/client/cli/test_kill.py
index 7aad34a..69d8402 100644
--- a/src/test/python/apache/aurora/client/cli/test_kill.py
+++ b/src/test/python/apache/aurora/client/cli/test_kill.py
@@ -26,12 +26,7 @@ from apache.aurora.client.cli.jobs import KillCommand
 from apache.aurora.client.cli.options import parse_instances, TaskInstanceKey
 from apache.aurora.common.aurora_job_key import AuroraJobKey
 
-from .util import (
-    AuroraClientCommandTest,
-    FakeAuroraCommandContext,
-    FakeAuroraCommandLine,
-    mock_verb_options
-)
+from .util import AuroraClientCommandTest, FakeAuroraCommandContext, mock_verb_options
 
 from gen.apache.aurora.api.constants import ACTIVE_STATES
 from gen.apache.aurora.api.ttypes import (
@@ -421,14 +416,12 @@ class TestClientKillCommand(AuroraClientCommandTest):
       with temporary_file() as fp:
         fp.write(self.get_valid_config())
         fp.flush()
-        cmd = FakeAuroraCommandLine()
+        cmd = AuroraCommandLine()
         cmd.execute(['job', 'kill', '--max-total-failures=1', '--config=%s' % fp.name,
                      '--batch-size=5', self.get_instance_spec('0,2,4-13')])
 
       assert mock_context.get_out() == []
       assert mock_context.get_err() == [
          'Instances [0, 2, 4, 5, 6] were not killed in time',
-         'Instances [7, 8, 9, 10, 11] were not killed in time']
-
-      assert cmd.get_err() == [
-        'Error executing command: Exceeded maximum number of errors while killing instances']
+         'Instances [7, 8, 9, 10, 11] were not killed in time',
+         'Error executing command: Exceeded maximum number of errors while killing instances']

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/test/python/apache/aurora/client/cli/test_supdate.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_supdate.py b/src/test/python/apache/aurora/client/cli/test_supdate.py
index 1806769..370a46b 100644
--- a/src/test/python/apache/aurora/client/cli/test_supdate.py
+++ b/src/test/python/apache/aurora/client/cli/test_supdate.py
@@ -335,7 +335,8 @@ class TestUpdateCommand(AuroraClientCommandTest):
         call(update_statuses=ACTIVE_JOB_UPDATE_STATES, job_key=self.TEST_JOBKEY)]
       assert mock_api.abort_job_update.mock_calls == []
       assert mock_context.get_out() == []
-      assert mock_context.get_err() == []
+      assert mock_context.get_err() == [
+        'Error executing command: scheduler returned multiple active updates for this job.']
 
   def test_pause_update_command_line_error(self):
     mock_context = FakeAuroraCommandContext()

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/7a6feee3/src/test/python/apache/aurora/client/cli/util.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/util.py b/src/test/python/apache/aurora/client/cli/util.py
index b65970a..864a714 100644
--- a/src/test/python/apache/aurora/client/cli/util.py
+++ b/src/test/python/apache/aurora/client/cli/util.py
@@ -17,7 +17,6 @@ import unittest
 
 from mock import create_autospec, Mock
 
-from apache.aurora.client.cli.client import AuroraCommandLine
 from apache.aurora.client.cli.context import AuroraCommandContext
 from apache.aurora.client.hooks.hooked_api import HookedAuroraClientAPI
 from apache.aurora.common.aurora_job_key import AuroraJobKey
@@ -25,7 +24,7 @@ from apache.aurora.common.aurora_job_key import AuroraJobKey
 from ...api_util import SchedulerProxyApiSpec, SchedulerThriftApiSpec
 from ..util import TEST_CLUSTER, TEST_CLUSTERS
 
-from gen.apache.aurora.api.constants import ACTIVE_STATES
+from gen.apache.aurora.api.constants import ACTIVE_STATES, CURRENT_API_VERSION
 from gen.apache.aurora.api.ttypes import (
     AssignedTask,
     ExecutorConfig,
@@ -38,6 +37,7 @@ from gen.apache.aurora.api.ttypes import (
     ScheduledTask,
     ScheduleStatus,
     ScheduleStatusResult,
+    ServerInfo,
     TaskConfig,
     TaskEvent,
     TaskQuery
@@ -57,19 +57,6 @@ def mock_verb_options(verb):
   return options
 
 
-class FakeAuroraCommandLine(AuroraCommandLine):
-  def __init__(self):
-    super(FakeAuroraCommandLine, self).__init__()
-    self.__err = []
-
-  def print_err(self, s, indent=0):
-    indent_str = " " * indent
-    self.__err.append("%s%s" % (indent_str, s))
-
-  def get_err(self):
-    return self.__err
-
-
 class FakeAuroraCommandContext(AuroraCommandContext):
   def __init__(self):
     super(FakeAuroraCommandContext, self).__init__()
@@ -136,7 +123,10 @@ class AuroraClientCommandTest(unittest.TestCase):
 
   @classmethod
   def create_blank_response(cls, code, msg):
-    return Response(responseCode=code, details=[ResponseDetail(message=msg)])
+    return Response(
+        responseCode=code,
+        details=[ResponseDetail(message=msg)],
+        serverInfo=ServerInfo(thriftAPIVersion=CURRENT_API_VERSION.major))
 
   @classmethod
   def create_simple_success_response(cls):


Mime
View raw message