How to make a custom condition?

[lanshark]

Hello All,

Trying to create a custom condition. Here is the code (which I have put in conditions.py in my django project, and imported.

@conditions.register('is_tester')
def is_tester_condition(request=None, **kwargs):
    """
    Django-Flags condition that requires an authcode passed in request, and
    determines if that authcode belongs to a tester, or is in a tester group.

    Returns True if yes, False otherwise.
    """

    logger.debug(f"is_tester_condition: kwargs: {kwargs}")
    authcode = request.GET.get("authCode", "").lower()
    if request is None:
        raise conditions.RequiredForCondition(
            "request is required for condition 'is_tester'"
        )
    if not authcode:
        raise conditions.RequiredForCondition(
            "authCode is required for condition 'is_tester'"
        )
    return True

I have another section of code that calls

for flag in get_flags():
    if flag_enabled (flag, request=request):
         do whatever

when this code runs, I get:

TypeError: is_tester_condition() got multiple values for argument 'request'

If I remove the request=None param from the condition definition, I get that the function takes 0 arguments, but is given 1.

Do you have an examples of custom conditions anywhere? Any ideas why it thinks request is being sent twice?

[lanshark]

Also - how do I create a condition that needs a parameter? How do I call that with flag_enabled? Some examples would be nice.

[willbarton]

@lanshark could you post the full traceback?

[lanshark]

# TypeError at /en-us/get_config: is_tester_condition() got multiple values for argument 'request'
Request Method: | GET

Traceback (most recent call last):
  File "/opt/poetry-venv/lib/python3.10/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/opt/poetry-venv/lib/python3.10/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/poetry-venv/lib/python3.10/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/opt/rmove/ms/views.py", line 2144, in get_config
    if flag_enabled(flag, request=request):
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/state.py", line 66, in flag_enabled
    return flag_state(flag_name, **kwargs)
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/state.py", line 61, in flag_state
    return _get_flag_state(flag_name, **kwargs)
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/state.py", line 13, in _get_flag_state
    return flag.check_state(**kwargs)
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/sources.py", line 53, in check_state
    checked_conditions = [(c, c.check(**kwargs)) for c in self.conditions]
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/sources.py", line 53, in <listcomp>
    checked_conditions = [(c, c.check(**kwargs)) for c in self.conditions]
  File "/opt/poetry-venv/lib/python3.10/site-packages/flags/sources.py", line 27, in check
    return self.fn(self.value, **kwargs)

Exception Type: TypeError at /en-us/get_config
Exception Value: is_tester_condition() got multiple values for argument 'request'

[lanshark]

These are the variables from the last step:

return self.fn(self.value, **kwargs)
…<a href="http://localhost:8000/en-us/get_config?authCode=xsrtest&amp;platform=iOS&amp;appVersion=2023.11.1
▼Local vars

Variable	Value
kwargs	{'request': <WSGIRequest: GET '/en-us/get_config?authCode=xsrtest&platform=iOS&appVersion=2023.11.1'>}
self	<flags.sources.Condition object at 0x402cfb1360>

[lanshark]

Here is the calling code:

Fetch feature flag values

feature_flags = {}
for flag in get_flags():
    if flag_enabled(flag, request=request):
        feature_flags[flag] = True
    else:
        feature_flags[flag] = False

[willbarton]

@lanshark Hmm. I think the problem is that Django-Flags always expects a condition to have a set "value" to evaluate a state-check against.

So, you've got a GET parameter-based condition, take a look at the "parameter" condition we already have:

@register("parameter", validator=validate_parameter)
def parameter_condition(param_name, request=None, **kwargs):
    """Is the parameter name part of the GET parameters?"""
    if request is None:
        raise RequiredForCondition(
            "request is required for condition 'parameter'"
        )
    try:
        param_name, param_value = param_name.split("=")
    except ValueError:
        param_value = "True"

    return request.GET.get(param_name) == param_value

Note that there's that initial value argument. Here, we use it as the name of the parameter to check for. State checks are always ultimately made against that value for any condition.

The problem here is that your condition doesn't require an initial value to check against — you have a specific parameter, and you seem to want to verify that it exists (and possibly do more with it that's not in the code you provided?), is that a correct reading on my part? What ends up happening is that the first argument, request, is taken as the condition's value, but it ends up also getting passed through as part of kwargs. So it's there twice.

Absent an API-change in Django-Flags (and right off, I'm not sure whether dropping a requirement for new conditions to have a value to compare against would be breaking or not), I think your best bet is to rewrite your condition such that it has a "value", and I'd probably recommend a simple boolean true/false. It's not ideal for your use-case, but it should work.

You'll see we do the same thing in our anonymous condition, another that strictly doesn't require any value to compare against, but for which the extra boolean provides a way to enable/disable the condition.

I'd envision something akin to the following:

from flags.utils import strtobool

@conditions.register('is_tester')
def is_tester_condition(boolean_enabled_value, request=None, **kwargs):
    if request is None:
        raise conditions.RequiredForCondition(
            "request is required for condition 'is_tester'"
        )
        
    authcode = request.GET.get("authCode", "").lower()
    enabled = strtobool(boolean_enabled_value.strip().lower()
    if not authcode and not enabled:
        raise conditions.RequiredForCondition(
            "authCode is required for condition 'is_tester'"
        )
    return True

And then when setting FLAGS in settings:

FLAGS = {
    {'condition': 'is_tester', 'value': 'true'},
}

Does that make sense? Does it help?

If so, I will also make sure we get the documentation updated to be clear that an initial value is required for conditions. When we're looking to make breaking changes for Django-Flags 6.x, I'll see if we can remove that requirement.

[lanshark]

Actually, that makes a lot of sense, and may in fact also answer my second question - how do I create a condition that needs a parameter. I was trying to make the simplest possible condition to be sure I got it working, before I added the more complicated steps (is_tester actually checks several different items of the logged in user - what group are they in, what study are they accessing, etc) to determine if the user is a tester.

I do think the documentation is a bit misleading - will see if I can find a way to improve it. Thanks for the assistance - will let you know if that works.

[lanshark]

OK, that raises another question - how is that 'value' passed to something like flag_enabled? Do I have to know that some flags require a parameter, and what that parameter is?

[willbarton]

how is that 'value' passed to something like flag_enabled?

The value comes from configuration (either FLAGS in settings or the FlagState if stored in the database). It is what flag_enabled is intended to check against.

The way flag state evaluation happens (flag_enabled()) is:

1. It looks up the flag
2. It checks each configured condition
3. Finally, it determines the state based on conditions that are required to be met (configured with 'required': True) and those that are not.

At step 2, when checking conditions, is when each condition's registered check function is called. That call is made using the configured value and then any keyword args passed to flag_enabled.

For example, consider the after date condition. If you have a flag configured with the value might be a specific ISO8601 date stamp (2024-01-02T20:18Z):

FLAGS = {
    'TEST_FLAG': [
        {'condition': 'after_date', 'value': '2024-01-02T20:18Z'},
    ],
}

So, basically flag_enabled("TEST_FLAG") would:

1. Get TEST_FLAG from settings
2. Loop through the list of conditions
3. Call the after_date_condition function with the. value from settings and with no other arguments, as none were given to flag_enabled.
4. The after_date_condition function checks the current date against the configured ISO8601 date stamp, returning current date > value date.
5. That result is returned to the caller of flag_enabled, because there are no other conditions to check.

---

Do I have to know that some flags require a parameter, and what that parameter is?

Unfortunately, yes—request is one such parameter. Not all flag conditions need a request to evaluate state. The simplest condition, boolean just evaluates to whatever the configured value is. However, Django-Flags requires any such parameters to be named and passed via keyword arguments. The first and only positional argument to a condition check function is the configured value. Everything else is a keyword argument.

That's why the RequiredForCondition exception exists, to let anyone performing a state check with flag_enabled (or flag_disabled) know when a parameter is required but was not given.

[lanshark]

OK, that seems to have solved my problems - I now have a working custom condition (actually, 4) and they seem to be working properly. Thanks!

[lanshark]

Actually, I spoke too soon. The custom conditions now don't throw errors, but apparently, the custom condition code is not being called.

At least, none of the log messages that I put inside the custom condition are ever logged, and the value of the flag does not change ever.

Suggestions on where to look @willbarton?

[willbarton]

@lanshark Do you mind posting the current code for the condition, how the flag is being configured to use it, and the code where you're checking the flag?

[lanshark]

They are defined in conf/conditions.py:

"""
This file defines custom RSG-specific conditions for django-flags.
"""

from flags import conditions


# NOTE: NONE of the custom conditions below are working...
# working with upstream to correct.


@conditions.register("is_tester")
def is_tester_condition(enabled: str = True, request=None, **kwargs):
    """
    Django-Flags condition that requires an authcode passed in request, and
    determines if that authcode belongs to a tester, or is in a tester group.
    The feature_flag is 'IS_TESTER'.

    Args:
        enabled: Is this condition enabled? (defaults to True)

        request: The http GET request from the client. Contains params:
            'authCode' to check

        **kwargs: any additional parameters

    Returns:
        True if the authCode is a tester or test group, False otherwise.
    """
    # this must be imported locally to avoid "apps not loaded" error
    from ms.views import _get_tester_and_group_from_code

    if request is None:
        msg = "request is required for condition 'is_tester'"
        raise conditions.RequiredForCondition(
            msg,
        )
    authcode = request.GET.get("authCode", "").strip().lower()
    if not authcode or not enabled:
        msg = "authCode is required for condition 'is_tester' or is_tester is disabled"
        raise conditions.RequiredForCondition(
            msg,
        )
    # call the check from authorize...
    (tester, group) = _get_tester_and_group_from_code(authcode)
    return tester


@conditions.register("platform")
def platform_condition(required_platform: str, request=None, **kwargs):
    """
    Django-Flags condition that requires a platform, and determines if the
    client platform matches.  There are 3 different feature_flags using this
    definition: 'PLATFORM_IOS', 'PLATFORM_ANDROID', and 'PLATFORM_BROWSER'.

    Args:
        required_platform: Either 'ios' or 'android' or 'browser'

        request: The http GET request from the client. Contains params:
            platform: The platform (iOS, Android, browser) (string)

        **kwargs: any additional parameters

    Returns:
        True if platform == required_platform, False otherwise.
    """
    platform = request.GET.get("platform", "").strip().lower()
    if not platform:
        msg = "platform is required for condition 'platform'"
        raise conditions.RequiredForCondition(
            msg,
        )
    return platform == required_platform


@conditions.register("app_version")
def app_version_condition(required_app_version: str, request=None, **kwargs):
    """
    Django-Flags condition that requires an app_version, and determines if the
    client app_version matches or is greater than.

    Args:
        required_app_version: CalVer version (string)

        request: The http GET request from the client. Contains params:
            appVersion: The CalVer-formatted client application version (string)

        **kwargs: any additional parameters

    Returns:
        True if app_version >= required_app_version, False otherwise.
    """
    app_version = request.GET.get("appVersion", "").strip().lower()
    if not app_version:
        msg = "app_version is required for condition 'app_version'"
        raise conditions.RequiredForCondition(
            msg,
        )
    # probably should call check_app_version...
    return app_version >= required_app_version
``

They are configured in conf/settings.py:

```python
if DEBUG:
    FLAG_STATE_LOGGING = True
FLAGS = {
    "EMPTY_FLAG": [],  # required for testing...
    "TRUE_FLAG": [  # required for testing...
        {"condition": "boolean", "value": True, "required": True},
    ],
    "FALSE_FLAG": [  # required for testing...
        {"condition": "boolean", "value": False, "required": True},
    ],
    "APP_VERSION_2023.10.0": [
            {"condition": "app_version", "value": "2023.10.0", "required": True},
    ],
    "DISABLE_PATHSENSE": [
        {"condition": "boolean", "value": True, "required": True},
    ],
    "DISABLE_RESTART": [
        {"condition": "boolean", "value": False, "required": True},
    ],
    "IS_TESTER": [
            {"condition": "is_tester", "value": True, "required": True},
    ],
    "PLATFORM_ANDROID": [
           {"condition": "platform", "value": "android", "required": True},
    ],
    "PLATFORM_IOS": [
            {"condition": "platform", "value": "ios", "required": True},
    ],
   "PLATFORM_BROWSER": [
           {"condition": "platform", "value": "browser", "required": True},
    ],
}

NOTE: DEBUG is set True, currently.

They are used HERE in ms/views.py, in an API call.

@csrf_exempt
def get_config(request):
    """
    An API to get feature flag values and configuration values
    for a given server.

    Args:
        request: The http GET request from the client. Contains params:

        authCode:  The user's authentication code (string)
        appVersion: The application's version in CalVer format. (string)
        platform: The platform (iOS, Android, browser) (string)

    Returns:
        json dictionary with 2 keys: "feature_flags" and "configuration".
    """
    logger.debug("get_config: start")

    # These must be present to be used in the custom condition code...
    authcode = request.GET.get("authCode", "").lower()
    app_version = request.GET.get("appVersion", "").lower()
    platform = request.GET.get("platform", "").lower()

    feature_flags = {}
    # only check feature_flags if all 3 were provided, otherwise return an empty
    # feature_flags dict.
    if authcode and app_version and platform:
        # Fetch feature flag values
        for flag in get_flags():
            if flag_enabled(
                flag,
                request=request,
                authCode=authcode,
                appVersion=app_version,
                platform=platform,
            ):
                feature_flags[flag] = True
            else:
                feature_flags[flag] = False

    configuration = {
        "server_version": VERSION,
    }

    data = {}
    data["feature_flags"] = feature_flags
    data["configuration"] = configuration

    logger.debug(f"get_config: returning {data}")
    return JsonResponse(data, safe=False)

I have confirmed that the get_flags call is being made, as it does return the TRUE_FLAG, etc.

Thanks for your help!

[willbarton]

Ok, so I think there are a couple things that I see that I'd note.

On a conceptual level, rather than having multiple flags with one condition each (or a mapping of 1:1 conditions-to-flag), I'd recommend creating one flag for each combination of conditions you want to check. Even when working as expected, for flag in get_flags(): flag_enabled() is going to be more expensive than a single flag_enabled() that checks multiple conditions. This way, you're enabling specific features based on the conditions, not checking for specific platforms, etc. Ideally, feature flags should be ephemeral to a certain degree.

This means I'd recommend a design more along the lines of:

FLAGS = {
    "IOS_FEATURE": [
            {"condition": "app_version", "value": "2023.10.0", "required": True},
            {"condition": "is_tester", "value": True, "required": True},
            {"condition": "platform", "value": "ios", "required": True},
    ],
}

The second thing is, when you're checking flags, you're checking for the existence of authCode, appVersion, and platform in request.GET unnecessarily if designing it as above. Your conditions are getting those values from request.GET directly (which I think is good), so flag_enabled("IOS_FEATURE", request=request) would achieve the same thing. The values aren't being used when passed in as keyword arguments.

You mention the messages not being logged, but I don't see any logging happening in the conditions, just in your get_config() view. Assuming you're logging DEBUG level messages, what is the output of your logger.debug(f"get_config: returning {data}")?

As a quick, simplified-for-brevity example, the following should work if pasted into a Django shell: (./manage.py shell):

from django.test import RequestFactory, override_settings
from flags import conditions
from flags.state import flag_enabled

@conditions.register("is_tester")
def is_tester_condition(enabled, request=None, **kwargs):
    authcode = request.GET.get("authCode", "")
    if not authcode:
        raise conditions.RequiredForCondition("code is required")
    return True

@conditions.register("platform")
def platform_condition(required_platform, request=None, **kwargs):
    platform = request.GET.get("platform", "")
    if not platform:
        raise conditions.RequiredForCondition("platform is required")
    return platform == required_platform

@conditions.register("app_version")
def app_version_condition(required_app_version, request=None, **kwargs):
    app_version = request.GET.get("appVersion", "")
    if not app_version:
        raise conditions.RequiredForCondition("version is required")
    return app_version >= required_app_version

with override_settings(FLAGS = {
    "IOS_FEATURE": [
            {"condition": "app_version", "value": "1", "required": True},
            {"condition": "is_tester", "value": True, "required": True},
            {"condition": "platform", "value": "ios", "required": True},
    ],
}):
    request = RequestFactory().get("/?authCode=foo&platform=ios&appVersion=1")
    flag_enabled("IOS_FEATURE", request=request)

That should return True — the flag is enabled. If you change the platform to something other than ios in the test request, it should return False:

with override_settings(FLAGS = {
    "IOS_FEATURE": [
            {"condition": "app_version", "value": "1", "required": True},
            {"condition": "is_tester", "value": True, "required": True},
            {"condition": "platform", "value": "ios", "required": True},
    ],
}):
    request = RequestFactory().get("/?authCode=foo&platform=browser&appVersion=1")
    flag_enabled("IOS_FEATURE", request=request)

And the same with the other values.

One quick thing to note — with "required": True in the configuration, they'll always be ANDed together that way.

If you remove that, they're ORed:

with override_settings(FLAGS = {
    "IOS_FEATURE": [
            {"condition": "app_version", "value": "1"},
            {"condition": "is_tester", "value": True},
            {"condition": "platform", "value": "ios"},
    ],
}):
    request = RequestFactory().get("/?authCode=foo&platform=browser")
   flag_enabled("IOS_FEATURE", request=request)

Should return True — because at least one condition out of the three passed.

(In general, an approach like this is how I think about creating unittests that help debug these kinds of things in isolation.)

---

I hope this helps a bit? I think your conditions look okay was implemented, but I'm not sure about the way you're designing the flags and trying to check them.