-
Notifications
You must be signed in to change notification settings - Fork 36
refact: Decide API #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refact: Decide API #314
Changes from 4 commits
a5accf0
c3d4e33
ac856c7
aa483cc
30da8fc
cc8dfda
98e99ff
7b5aa3a
6c29ee8
c9a0e64
0462e72
fb5cef9
fa3f6d9
3732d84
76666cf
a84e53a
83aa045
d355150
675408a
c7c93ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,6 @@ | |
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
import logging | ||
import threading | ||
|
||
from six import string_types | ||
|
||
|
@@ -53,7 +51,7 @@ def __init__( | |
notification_center=None, | ||
event_processor=None, | ||
datafile_access_token=None, | ||
default_decisions=None | ||
default_decide_options=None | ||
): | ||
""" Optimizely init method for managing Custom projects. | ||
|
||
|
@@ -77,7 +75,7 @@ def __init__( | |
which simply forwards events to the event dispatcher. | ||
To enable event batching configure and use optimizely.event.event_processor.BatchEventProcessor. | ||
datafile_access_token: Optional string used to fetch authenticated datafile for a secure project environment. | ||
default_decisions: Optional list of decide options used with the decide APIs. | ||
default_decide_options: Optional list of decide options used with the decide APIs. | ||
""" | ||
self.logger_name = '.'.join([__name__, self.__class__.__name__]) | ||
self.is_valid = True | ||
|
@@ -89,7 +87,13 @@ def __init__( | |
self.event_processor = event_processor or ForwardingEventProcessor( | ||
self.event_dispatcher, logger=self.logger, notification_center=self.notification_center, | ||
) | ||
self.default_decisions = default_decisions or [] | ||
|
||
if default_decide_options is None: | ||
self.default_decide_options = [] | ||
|
||
if not isinstance(self.default_decide_options, list): | ||
self.logger.debug('Provided default decide options is not a list.') | ||
self.default_decide_options = [] | ||
|
||
try: | ||
self._validate_instantiation_options() | ||
|
@@ -203,8 +207,7 @@ def _send_impression_event(self, project_config, experiment, variation, flag_key | |
) | ||
|
||
def _get_feature_variable_for_type( | ||
self, project_config, feature_key, variable_key, variable_type, user_id, attributes, | ||
ignore_user_profile=False | ||
self, project_config, feature_key, variable_key, variable_type, user_id, attributes | ||
): | ||
""" Helper method to determine value for a certain variable attached to a feature flag based on type of variable. | ||
|
||
|
@@ -215,7 +218,6 @@ def _get_feature_variable_for_type( | |
variable_type: Type of variable which could be one of boolean/double/integer/string. | ||
user_id: ID for user. | ||
attributes: Dict representing user attributes. | ||
ignore_user_profile: if true don't use the user profile service | ||
|
||
Returns: | ||
Value of the variable. None if: | ||
|
@@ -259,7 +261,7 @@ def _get_feature_variable_for_type( | |
source_info = {} | ||
variable_value = variable.defaultValue | ||
decision = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_id, | ||
attributes, ignore_user_profile) | ||
attributes) | ||
if decision.variation: | ||
|
||
feature_enabled = decision.variation.featureEnabled | ||
|
@@ -310,7 +312,7 @@ def _get_feature_variable_for_type( | |
return actual_value | ||
|
||
def _get_all_feature_variables_for_type( | ||
self, project_config, feature_key, user_id, attributes, | ||
self, project_config, feature_key, user_id, attributes, | ||
): | ||
""" Helper method to determine value for all variables attached to a feature flag. | ||
|
||
|
@@ -948,8 +950,7 @@ def create_user_context(self, user_id, attributes=None): | |
self.logger.error(enums.Errors.INVALID_INPUT.format('attributes')) | ||
return None | ||
|
||
user_context = UserContext(self, user_id, attributes) | ||
return user_context | ||
return UserContext(self, user_id, attributes) | ||
|
||
def decide(self, user_context, key, decide_options=None): | ||
""" | ||
|
@@ -971,15 +972,15 @@ def decide(self, user_context, key, decide_options=None): | |
|
||
# check if SDK is ready | ||
if not self.is_valid: | ||
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('decide')) | ||
self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('decide')) | ||
reasons.append(DecisionMessage.SDK_NOT_READY) | ||
return Decision(flag_key=key, user_context=user_context, reasons=reasons) | ||
|
||
# validate that key is a string | ||
if not isinstance(key, string_types): | ||
self.logger.error('Key parameter is invalid') | ||
reasons.append(DecisionMessage.FLAG_KEY_INVALID.format(key)) | ||
return Decision.new(flag_key=key, user_context=user_context, reasons=reasons) | ||
return Decision(flag_key=key, user_context=user_context, reasons=reasons) | ||
|
||
# validate that key maps to a feature flag | ||
config = self.config_manager.get_config() | ||
|
@@ -996,38 +997,10 @@ def decide(self, user_context, key, decide_options=None): | |
|
||
# merge decide_options and default_decide_options | ||
if isinstance(decide_options, list): | ||
decide_options += self.default_decisions | ||
decide_options += self.default_decide_options | ||
else: | ||
self.logger.debug('Provided decide options is not an array. Using default decide options.') | ||
decide_options = self.default_decisions | ||
|
||
class ReasonLogHandler(logging.StreamHandler): | ||
def __init__(self): | ||
super(ReasonLogHandler, self).__init__() | ||
self._name = "ReasonLogHandler" | ||
self.reasons = {threading.current_thread().ident: []} | ||
# setting to info level since we don't put debug in reasons. | ||
self.level = logging.INFO | ||
formatter = logging.Formatter('%(levelname)-8s %(asctime)s %(filename)s:%(lineno)s:%(message)s') | ||
self.setFormatter(formatter) | ||
self.createLock() | ||
|
||
def handle(self, record): | ||
msg = self.format(record) | ||
self.reasons[threading.current_thread().ident].append(msg) | ||
|
||
def emit(self, record): | ||
pass | ||
|
||
def get_reasons(self): | ||
return self.reasons[threading.current_thread().ident] | ||
|
||
handler = None | ||
|
||
if DecideOption.INCLUDE_REASONS in decide_options: | ||
handler = ReasonLogHandler() | ||
self.decision_service.logger.addHandler(handler) | ||
config.logger.addHandler(handler) | ||
decide_options = self.default_decide_options | ||
|
||
# Create Optimizely Decision Result. | ||
user_id = user_context.user_id | ||
|
@@ -1041,6 +1014,7 @@ def get_reasons(self): | |
experiment = None | ||
decision_source = DecisionSources.ROLLOUT | ||
source_info = {} | ||
decision_event_dispatched = False | ||
|
||
decision = self.decision_service.get_variation_for_feature(config, feature_flag, user_context.user_id, | ||
user_context.user_attributes, | ||
|
@@ -1064,45 +1038,51 @@ def get_reasons(self): | |
if DecideOption.DISABLE_DECISION_EVENT not in decide_options: | ||
if decision_source == DecisionSources.FEATURE_TEST or config.send_flag_decisions: | ||
self._send_impression_event(config, experiment, variation, flag_key, rule_key or '', | ||
feature_enabled, decision_source, | ||
decision_source, feature_enabled, | ||
user_id, attributes) | ||
decision_event_dispatched = True | ||
|
||
# Generate all variables map if decide options doesn't include excludeVariables | ||
if DecideOption.EXCLUDE_VARIABLES not in decide_options: | ||
project_config = self.config_manager.get_config() | ||
for v_key in feature_flag.variables: | ||
v = feature_flag.variables[v_key] | ||
all_variables[v.key] = self._get_feature_variable_for_type(project_config, feature_flag.key, | ||
v.key, v.type, user_id, attributes, | ||
DecideOption.IGNORE_USER_PROFILE_SERVICE in | ||
decide_options | ||
) | ||
for variable_key in feature_flag.variables: | ||
variable = config.get_variable_for_feature(flag_key, variable_key) | ||
variable_value = variable.defaultValue | ||
if feature_enabled: | ||
variable_value = config.get_variable_value_for_variation(variable, decision.variation) | ||
self.logger.debug( | ||
'Got variable value "%s" for variable "%s" of feature flag "%s".' | ||
% (variable_value, variable_key, flag_key) | ||
) | ||
|
||
try: | ||
actual_value = config.get_typecast_value(variable_value, variable.type) | ||
except: | ||
self.logger.error('Unable to cast value. Returning None.') | ||
actual_value = None | ||
|
||
all_variables[variable_key] = actual_value | ||
|
||
# Send notification | ||
self.notification_center.send_notifications( | ||
enums.NotificationTypes.DECISION, | ||
enums.DecisionNotificationTypes.FEATURE, | ||
enums.DecisionNotificationTypes.FLAG, | ||
user_id, | ||
attributes or {}, | ||
{ | ||
'feature_key': key, | ||
'feature_enabled': feature_enabled, | ||
'source': decision.source, | ||
'source_info': source_info, | ||
'flag_key': flag_key, | ||
'enabled': feature_enabled, | ||
'variables': all_variables, | ||
'variation_key': variation_key, | ||
'rule_key': rule_key, | ||
'reasons': reasons, | ||
'decision_event_dispatched': decision_event_dispatched | ||
|
||
}, | ||
) | ||
|
||
include_reasons = [] | ||
if DecideOption.INCLUDE_REASONS in decide_options: | ||
handler.flush() | ||
include_reasons = reasons | ||
include_reasons += handler.get_reasons() | ||
self.decision_service.logger.removeHandler(handler) | ||
config.logger.removeHandler(handler) | ||
|
||
return Decision(variation_key=variation_key, enabled=feature_enabled, variables=all_variables, | ||
rule_key=rule_key, | ||
flag_key=flag_key, user_context=user_context, reasons=include_reasons) | ||
flag_key=flag_key, user_context=user_context, reasons=reasons) | ||
|
||
def decide_all(self, user_context, decide_options=None): | ||
""" | ||
|
@@ -1120,7 +1100,7 @@ def decide_all(self, user_context, decide_options=None): | |
|
||
# check if SDK is ready | ||
if not self.is_valid: | ||
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('decide_all')) | ||
self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('decide_all')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. line #1131: return {} when SDK_NOT_READY There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it already does. Looks like github is not showing you the updated diff. |
||
return {} | ||
|
||
config = self.config_manager.get_config() | ||
|
@@ -1133,7 +1113,6 @@ def decide_all(self, user_context, decide_options=None): | |
keys = [] | ||
for f in config.feature_flags: | ||
keys.append(f['key']) | ||
|
||
return self.decide_for_keys(user_context, keys, decide_options) | ||
|
||
def decide_for_keys(self, user_context, keys, decide_options=None): | ||
|
@@ -1153,7 +1132,7 @@ def decide_for_keys(self, user_context, keys, decide_options=None): | |
|
||
# check if SDK is ready | ||
if not self.is_valid: | ||
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('decide_for_keys')) | ||
self.logger.error(enums.Errors.INVALID_OPTIMIZELY.format('decide_for_keys')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. line #1161: should check with "default_decide_options" as well |
||
return {} | ||
|
||
enabled_flags_only = False | ||
|
@@ -1166,5 +1145,4 @@ def decide_for_keys(self, user_context, keys, decide_options=None): | |
if enabled_flags_only and not decision.enabled: | ||
continue | ||
decisions[key] = decision | ||
|
||
return decisions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if default_decide_option is non-null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already placed another check below which ensures that it must be a list when non-null