Skip to content

Commit c15bfaf

Browse files
mredolattimredolatti
committed
Add tests for algo field. Fixed bug that resulted in algo=2 being treated as legacy hash
1 parent 77764ce commit c15bfaf

File tree

7 files changed

+317
-20
lines changed

7 files changed

+317
-20
lines changed

‎splitio/hashfns/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from __future__ import absolute_import, division, print_function, \
77
unicode_literals
88

9+
from splitio.splits import HashAlgorithm
910
from splitio.hashfns import legacy
1011

1112
try:
@@ -23,8 +24,8 @@ def _murmur_hash(key, seed):
2324

2425

2526
_HASH_ALGORITHMS = {
26-
'legacy': legacy.legacy_hash,
27-
'murmur': _murmur_hash
27+
HashAlgorithm.LEGACY: legacy.legacy_hash,
28+
HashAlgorithm.MURMUR: _murmur_hash
2829
}
2930

3031

‎splitio/redis_support.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def missing_redis_dependencies(*args, **kwargs):
2323
from splitio.matchers import UserDefinedSegmentMatcher
2424
from splitio.metrics import BUCKETS
2525
from splitio.segments import Segment
26-
from splitio.splits import Split, SplitParser, HashAlgorithm
26+
from splitio.splits import Split, SplitParser
2727
from splitio.impressions import Impression
2828
from splitio.utils import bytes_to_string
2929

@@ -202,7 +202,6 @@ def add_split(self, split_name, split):
202202
def get_split(self, split_name):
203203

204204
to_decode = self._redis.get(self._get_split_key(split_name))
205-
206205
if to_decode is None:
207206
return None
208207

@@ -665,9 +664,12 @@ def _parse_matcher_in_segment(self, partial_split, matcher, block_until_ready=Fa
665664

666665

667666
class RedisSplit(Split):
668-
def __init__(self, name, seed, killed, default_treatment, traffic_type_name, status, change_number, conditions=None, segment_cache=None, algo=HashAlgorithm.LEGACY):
669-
"""A split implementation that mantains a reference to the segment cache so segments can
670-
be easily pickled and unpickled.
667+
def __init__(self, name, seed, killed, default_treatment, traffic_type_name,
668+
status, change_number, conditions=None, segment_cache=None,
669+
algo=None):
670+
'''
671+
A split implementation that mantains a reference to the segment cache
672+
so segments can be easily pickled and unpickled.
671673
:param name: Name of the feature
672674
:type name: unicode
673675
:param seed: Seed
@@ -680,8 +682,10 @@ def __init__(self, name, seed, killed, default_treatment, traffic_type_name, sta
680682
:type conditions: list
681683
:param segment_cache: A segment cache
682684
:type segment_cache: SegmentCache
683-
"""
684-
super(RedisSplit, self).__init__(name, seed, killed, default_treatment, traffic_type_name, status, change_number, conditions)
685+
'''
686+
super(RedisSplit, self).__init__(name, seed, killed, default_treatment,
687+
traffic_type_name, status,
688+
change_number, conditions, algo)
685689
self._segment_cache = segment_cache
686690

687691
@property

‎splitio/splits.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,13 @@ class HashAlgorithm(Enum):
3434
"""
3535
Hash algorithm names
3636
"""
37-
LEGACY = "legacy"
38-
MURMUR = "murmur"
37+
LEGACY = 1
38+
MURMUR = 2
3939

4040

4141
class Split(object):
4242
def __init__(self, name, seed, killed, default_treatment, traffic_type_name,
43-
status, change_number, conditions=None,
44-
algo=HashAlgorithm.LEGACY):
43+
status, change_number, conditions=None, algo=None):
4544
"""
4645
A class that represents a split. It associates a feature name with a set
4746
of matchers (responsible of telling which condition to use) and
@@ -65,7 +64,7 @@ def __init__(self, name, seed, killed, default_treatment, traffic_type_name,
6564
self._status = status
6665
self._change_number = change_number
6766
self._conditions = conditions if conditions is not None else []
68-
self._algo = algo
67+
self._algo = HashAlgorithm(algo) if algo else HashAlgorithm.LEGACY
6968

7069
@property
7170
def name(self):

‎splitio/tests/algoSplits.json

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
{
2+
"splits": [
3+
{
4+
"orgId": null,
5+
"environment": null,
6+
"trafficTypeId": null,
7+
"trafficTypeName": null,
8+
"name": "some_feature_1",
9+
"changeNumber":1325599980,
10+
"algo": 1,
11+
"seed": -1222652054,
12+
"status": "ACTIVE",
13+
"killed": false,
14+
"defaultTreatment": "off",
15+
"conditions": [
16+
{
17+
"matcherGroup": {
18+
"combiner": "AND",
19+
"matchers": [
20+
{
21+
"matcherType": "WHITELIST",
22+
"negate": false,
23+
"userDefinedSegmentMatcherData": null,
24+
"whitelistMatcherData": {
25+
"whitelist": [
26+
"whitelisted_user"
27+
]
28+
}
29+
}
30+
]
31+
},
32+
"partitions": [
33+
{
34+
"treatment": "on",
35+
"size": 100
36+
}
37+
]
38+
},
39+
{
40+
"matcherGroup": {
41+
"combiner": "AND",
42+
"matchers": [
43+
{
44+
"matcherType": "ALL_KEYS",
45+
"negate": false,
46+
"userDefinedSegmentMatcherData": null,
47+
"whitelistMatcherData": null
48+
}
49+
]
50+
},
51+
"partitions": [
52+
{
53+
"treatment": "on",
54+
"size": 0
55+
},
56+
{
57+
"treatment": "off",
58+
"size": 100
59+
}
60+
]
61+
}
62+
]
63+
},
64+
{
65+
"orgId": null,
66+
"environment": null,
67+
"trafficTypeId": null,
68+
"trafficTypeName": null,
69+
"name": "some_feature_2",
70+
"algo": 2,
71+
"changeNumber":1325599980,
72+
"seed": 1699838640,
73+
"status": "ACTIVE",
74+
"killed": false,
75+
"defaultTreatment": "off",
76+
"conditions": [
77+
{
78+
"matcherGroup": {
79+
"combiner": "AND",
80+
"matchers": [
81+
{
82+
"matcherType": "ALL_KEYS",
83+
"negate": false,
84+
"userDefinedSegmentMatcherData": null,
85+
"whitelistMatcherData": null
86+
}
87+
]
88+
},
89+
"partitions": [
90+
{
91+
"treatment": "on",
92+
"size": 100
93+
},
94+
{
95+
"treatment": "off",
96+
"size": 0
97+
}
98+
]
99+
}
100+
]
101+
},
102+
{
103+
"orgId": null,
104+
"environment": null,
105+
"trafficTypeId": null,
106+
"trafficTypeName": null,
107+
"name": "some_feature_3",
108+
"algo": null,
109+
"changeNumber":1325599980,
110+
"seed": -480091424,
111+
"status": "ACTIVE",
112+
"killed": true,
113+
"defaultTreatment": "defTreatment",
114+
"conditions": [
115+
{
116+
"matcherGroup": {
117+
"combiner": "AND",
118+
"matchers": [
119+
{
120+
"matcherType": "ALL_KEYS",
121+
"negate": false,
122+
"userDefinedSegmentMatcherData": null,
123+
"whitelistMatcherData": null
124+
}
125+
]
126+
},
127+
"partitions": [
128+
{
129+
"treatment": "defTreatment",
130+
"size": 100
131+
},
132+
{
133+
"treatment": "off",
134+
"size": 0
135+
}
136+
]
137+
}
138+
]
139+
},
140+
{
141+
"orgId": null,
142+
"environment": null,
143+
"trafficTypeId": null,
144+
"trafficTypeName": null,
145+
"name": "some_feature_4",
146+
"seed": 1548363147,
147+
"changeNumber":1325599980,
148+
"status": "ACTIVE",
149+
"killed": false,
150+
"defaultTreatment": "off",
151+
"conditions": [
152+
{
153+
"matcherGroup": {
154+
"combiner": "AND",
155+
"matchers": [
156+
{
157+
"matcherType": "IN_SEGMENT",
158+
"negate": false,
159+
"userDefinedSegmentMatcherData": {
160+
"segmentName": "employees"
161+
},
162+
"whitelistMatcherData": null
163+
}
164+
]
165+
},
166+
"partitions": [
167+
{
168+
"treatment": "on",
169+
"size": 100
170+
}
171+
]
172+
},
173+
{
174+
"matcherGroup": {
175+
"combiner": "AND",
176+
"matchers": [
177+
{
178+
"matcherType": "IN_SEGMENT",
179+
"negate": false,
180+
"userDefinedSegmentMatcherData": {
181+
"segmentName": "human_beigns"
182+
},
183+
"whitelistMatcherData": null
184+
}
185+
]
186+
},
187+
"partitions": [
188+
{
189+
"treatment": "on",
190+
"size": 30
191+
},
192+
{
193+
"treatment": "off",
194+
"size": 70
195+
}
196+
]
197+
}
198+
]
199+
}
200+
],
201+
"since": -1,
202+
"till": 1457726098069
203+
}

0 commit comments

Comments
 (0)