From 67e99991d52bc788a1380e8cb5fa1ffa27a484c8 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Fri, 28 Feb 2014 11:22:26 +1000 Subject: [PATCH] Add a positional decorator This decorator allow us to define that certain parameters are strictly keyword arguments only, without sacrificing the convenience of having those arguments listed in the function definition. This will mean that we are no longer trapped maintaining backwards compatibility for the order of keyword arguments. The enforcement levels should allow us to start putting warnings on existing functions and strict enforcement on new functions. Partial-Bug: #1295881 Change-Id: Ic0a196874930e7be8362df50594471ab8037c5b2 --- keystoneclient/tests/test_utils.py | 73 +++++++++++++ keystoneclient/utils.py | 161 +++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+) diff --git a/keystoneclient/tests/test_utils.py b/keystoneclient/tests/test_utils.py index 27d3c8d79..909fe9344 100644 --- a/keystoneclient/tests/test_utils.py +++ b/keystoneclient/tests/test_utils.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import logging import sys import six @@ -131,3 +132,75 @@ class PrintTestCase(test_utils.TestCase): if isinstance(output, six.binary_type): output = output.decode('utf-8') self.assertIn(name, output) + + +class TestPositional(test_utils.TestCase): + + @utils.positional(1) + def no_vars(self): + # positional doesn't enforce anything here + return True + + @utils.positional(3, utils.positional.EXCEPT) + def mixed_except(self, arg, kwarg1=None, kwarg2=None): + # self, arg, and kwarg1 may be passed positionally + return (arg, kwarg1, kwarg2) + + @utils.positional(3, utils.positional.WARN) + def mixed_warn(self, arg, kwarg1=None, kwarg2=None): + # self, arg, and kwarg1 may be passed positionally, only a warning + # is emitted + return (arg, kwarg1, kwarg2) + + def test_nothing(self): + self.assertTrue(self.no_vars()) + + def test_mixed_except(self): + self.assertEqual((1, 2, 3), self.mixed_except(1, 2, kwarg2=3)) + self.assertEqual((1, 2, 3), self.mixed_except(1, kwarg1=2, kwarg2=3)) + self.assertEqual((1, None, None), self.mixed_except(1)) + self.assertRaises(TypeError, self.mixed_except, 1, 2, 3) + + def test_mixed_warn(self): + logger_message = six.moves.cStringIO() + handler = logging.StreamHandler(logger_message) + handler.setLevel(logging.DEBUG) + + logger = logging.getLogger(utils.__name__) + level = logger.getEffectiveLevel() + logger.setLevel(logging.DEBUG) + logger.addHandler(handler) + + self.addCleanup(logger.removeHandler, handler) + self.addCleanup(logger.setLevel, level) + + self.mixed_warn(1, 2, 3) + + self.assertIn('takes at most 3 positional', logger_message.getvalue()) + + @utils.positional(enforcement=utils.positional.EXCEPT) + def inspect_func(self, arg, kwarg=None): + return (arg, kwarg) + + def test_inspect_positions(self): + self.assertEqual((1, None), self.inspect_func(1)) + self.assertEqual((1, 2), self.inspect_func(1, kwarg=2)) + self.assertRaises(TypeError, self.inspect_func) + self.assertRaises(TypeError, self.inspect_func, 1, 2) + + @utils.positional.classmethod(1) + def class_method(cls, a, b): + return (cls, a, b) + + @utils.positional.method(1) + def normal_method(self, a, b): + self.assertIsInstance(self, TestPositional) + return (self, a, b) + + def test_class_method(self): + self.assertEqual((TestPositional, 1, 2), self.class_method(1, b=2)) + self.assertRaises(TypeError, self.class_method, 1, 2) + + def test_normal_method(self): + self.assertEqual((self, 1, 2), self.normal_method(1, b=2)) + self.assertRaises(TypeError, self.normal_method, 1, 2) diff --git a/keystoneclient/utils.py b/keystoneclient/utils.py index 7b41a4b61..00d58b76c 100644 --- a/keystoneclient/utils.py +++ b/keystoneclient/utils.py @@ -10,8 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. +import functools import getpass import hashlib +import inspect +import logging import sys import prettytable @@ -21,6 +24,9 @@ from keystoneclient import exceptions from keystoneclient.openstack.common import strutils +logger = logging.getLogger(__name__) + + # Decorator for cli-args def arg(*args, **kwargs): def _decorator(func): @@ -157,3 +163,158 @@ def prompt_for_password(): return new_passwd except EOFError: return + + +class positional(object): + """A decorator which enforces only some args may be passed positionally. + + This idea and some of the code was taken from the oauth2 client of the + google-api client. + + This decorator makes it easy to support Python 3 style key-word only + parameters. For example, in Python 3 it is possible to write:: + + def fn(pos1, *, kwonly1, kwonly2=None): + ... + + All named parameters after * must be a keyword:: + + fn(10, 'kw1', 'kw2') # Raises exception. + fn(10, kwonly1='kw1', kwonly2='kw2') # Ok. + + To replicate this behaviour with the positional decorator you simply + specify how many arguments may be passed positionally. To replicate the + example above:: + + @positional(1) + def fn(pos1, kwonly1=None, kwonly2=None): + ... + + If no default value is provided to a keyword argument, it becomes a + required keyword argument:: + + @positional(0) + def fn(required_kw): + ... + + This must be called with the keyword parameter:: + + fn() # Raises exception. + fn(10) # Raises exception. + fn(required_kw=10) # Ok. + + When defining instance or class methods always remember that in python the + first positional argument passed is always the instance so you will need to + account for `self` and `cls`:: + + class MyClass(object): + + @positional(2) + def my_method(self, pos1, kwonly1=None): + ... + + @classmethod + @positional(2) + def my_method(cls, pos1, kwonly1=None): + ... + + If you would prefer not to account for `self` and `cls` you can use the + `method` and `classmethod` helpers which do not consider the initial + positional argument. So the following class is exactly the same as the one + above:: + + class MyClass(object): + + @positional.method(1) + def my_method(self, pos1, kwonly1=None): + ... + + @positional.classmethod(1) + def my_method(cls, pos1, kwonly1=None): + ... + + If a value isn't provided to the decorator then it will enforce that + every variable without a default value will be required to be a kwarg:: + + @positional() + def fn(pos1, kwonly1=None): + ... + + fn(10) # Ok. + fn(10, 20) # Raises exception. + fn(10, kwonly1=20) # Ok. + + This behaviour will work with the `positional.method` and + `positional.classmethod` helper functions as well:: + + class MyClass(object): + + @positional.classmethod() + def my_method(cls, pos1, kwonly1=None): + ... + + MyClass.my_method(10) # Ok. + MyClass.my_method(10, 20) # Raises exception. + MyClass.my_method(10, kwonly1=20) # Ok. + + For compatibility reasons you may wish to not always raise an exception so + a WARN mode is available. Rather than raise an exception a warning message + will be logged:: + + @positional(1, enforcement=positional.WARN): + def fn(pos1, kwonly=1): + ... + + Available modes are: + + - positional.EXCEPT - the default, raise an exception. + - positional.WARN - log a warning on mistake. + """ + + EXCEPT = 'except' + WARN = 'warn' + + def __init__(self, max_positional_args=None, enforcement=EXCEPT): + self._max_positional_args = max_positional_args + self._enforcement = enforcement + + @classmethod + def method(cls, max_positional_args=None, enforcement=EXCEPT): + if max_positional_args is not None: + max_positional_args += 1 + + def f(func): + return cls(max_positional_args, enforcement)(func) + return f + + @classmethod + def classmethod(cls, *args, **kwargs): + def f(func): + return classmethod(cls.method(*args, **kwargs)(func)) + return f + + def __call__(self, func): + if self._max_positional_args is None: + spec = inspect.getargspec(func) + self._max_positional_args = len(spec.args) - len(spec.defaults) + + plural = '' if self._max_positional_args == 1 else 's' + + @functools.wraps(func) + def inner(*args, **kwargs): + if len(args) > self._max_positional_args: + message = ('%(name)s takes at most %(max)d positional ' + 'argument%(plural)s (%(given)d given)' % + {'name': func.__name__, + 'max': self._max_positional_args, + 'given': len(args), + 'plural': plural}) + + if self._enforcement == self.EXCEPT: + raise TypeError(message) + elif self._enforcement == self.WARN: + logger.warn(message) + + return func(*args, **kwargs) + + return inner