From 41f1401baf660afba0a52d8446d538172eb25a9b Mon Sep 17 00:00:00 2001 From: sundeep yedida Date: Tue, 7 Jun 2016 14:29:22 -0700 Subject: [PATCH] Update validity check for experiment names to be case sensitive The sixpack-py client allows for experiment names to be uppercased, but the javascript and ruby clients enforce lowecase experiment names only. This PR updates the client code to match the javascript and ruby implementations. + Add another regex for experiment name and use that for validation + Add tests --- sixpack/sixpack.py | 10 ++++++++-- sixpack/test/test_sixpack_client.py | 12 +++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/sixpack/sixpack.py b/sixpack/sixpack.py index 92bed05..e940093 100644 --- a/sixpack/sixpack.py +++ b/sixpack/sixpack.py @@ -4,6 +4,12 @@ SIXPACK_HOST = 'http://localhost:5000' SIXPACK_TIMEOUT = 0.5 + +# valid experiment_names must be lowercase, start with an alphanumeric and +# contain alphanumerics, dashes and underscores +VALID_EXPT_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9\-_ ]*$") + +# Valid alternative and kpi names VALID_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9\-_ ]*$", re.I) @@ -42,7 +48,7 @@ def __init__(self, client_id=None, options=None, params=None): self.client_id = client_id def participate(self, experiment_name, alternatives, force=None, traffic_fraction=1, prefetch=False): - if VALID_NAME_RE.match(experiment_name) is None: + if VALID_EXPT_NAME_RE.match(experiment_name) is None: raise ValueError('Bad experiment name') if len(alternatives) < 2: @@ -72,7 +78,7 @@ def participate(self, experiment_name, alternatives, force=None, traffic_fractio return response def convert(self, experiment_name, kpi=None): - if VALID_NAME_RE.match(experiment_name) is None: + if VALID_EXPT_NAME_RE.match(experiment_name) is None: raise ValueError('Bad experiment name') params = { diff --git a/sixpack/test/test_sixpack_client.py b/sixpack/test/test_sixpack_client.py index 74711f5..585dd3f 100644 --- a/sixpack/test/test_sixpack_client.py +++ b/sixpack/test/test_sixpack_client.py @@ -59,7 +59,7 @@ def test_client_id_on_constructor(self): session = sixpack.Session(client_id='zack111') self.assertEqual(session.client_id, 'zack111') - def test_failure_on_bad_experiment_name(self): + def test_failure_on_bad_experiment_name_participate(self): session = sixpack.Session('zack') with self.assertRaises(ValueError): session.participate('*********', ['1', '2']) @@ -69,6 +69,16 @@ def test_failure_on_bad_exp_name_convert(self): with self.assertRaises(ValueError): session.convert('(((((') + def test_failure_on_uppercase_expt_name_participate(self): + session = sixpack.Session('zack') + with self.assertRaises(ValueError): + session.participate('FooExperiment', ['1', '2']) + + def test_failure_on_uppercase_expt_name_convert(self): + session = sixpack.Session('zack') + with self.assertRaises(ValueError): + session.convert('FooExperiment') + def test_failure_on_bad_kpi_name_convert(self): session = sixpack.Session('zack') with self.assertRaises(ValueError):