Skip to content

[WIP] Extend ab test capabilities#1

Open
nacosta wants to merge 12 commits into
masterfrom
extend_ab_test_capabilities
Open

[WIP] Extend ab test capabilities#1
nacosta wants to merge 12 commits into
masterfrom
extend_ab_test_capabilities

Conversation

@nacosta

@nacosta nacosta commented Oct 25, 2018

Copy link
Copy Markdown

Modified the Gargoyle package to accept a flag for A/B testing

class UserConditionSetTests(TestCase):
""" Regression tests (before adding AB_TEST) """

def _create_contition(self, name, condition):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in contition

Comment thread gargoyle/builtins.py
if condition is not None:
return bool(condition)
return None
if isinstance(instance, AnonymousUser):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that you modified this because the UserConditionSet overrides the is_active and you passed in the switch_type but has it been refactored as well in the same time? Because is hard to understand if the logic has changed. Is hard to review this bit...

Comment thread gargoyle/manager.py
>>> gargoyle.is_active('my_feature', request)
"""
default = kwargs.pop('default', False)
flag_type = kwargs.pop('flag_type', FEATURE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep the naming consistent to switch_type

Comment thread gargoyle/conditions.py
field_condition = (*field_condition, FEATURE)
status, condition, condition_type = field_condition

if switch_type != condition_type: # Ignore condition with no switch type

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would this situation happen?

Comment thread gargoyle/conditions.py
return value

def has_active_condition(self, conditions, instances):
def has_active_condition(self, conditions, instances, switch_type=FEATURE):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use condition_type instead of switch_type

""" Returns an empty object which can get new fields in execution time """
return type("", (), {})

def _create_contitions(self, conditions):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo...

@nacosta nacosta changed the title Extend ab test capabilities [WIP] Extend ab test capabilities Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants