Skip to content

Commit

Permalink
Update validity check for experiment names to be case sensitive
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sundeep yedida committed Jun 7, 2016
1 parent af1c7a8 commit 41f1401
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
10 changes: 8 additions & 2 deletions sixpack/sixpack.py
Expand Up @@ -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)


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 = {
Expand Down
12 changes: 11 additions & 1 deletion sixpack/test/test_sixpack_client.py
Expand Up @@ -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'])
Expand All @@ -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):
Expand Down

0 comments on commit 41f1401

Please sign in to comment.