adamchainz / gargoyle

:radio_button: Feature switches in Django
Apache License 2.0
109 stars 19 forks source link

Add a switch to bypass decorator functionality #166

Closed Timvde closed 6 years ago

Timvde commented 6 years ago

I have created this small snippet in our codebase:

def toggle_decorator(gargoyle_switch, decorator):
    def _outer_wrapper(func):
        @wraps(func)
        def _inner_wrapper(*args, **kwargs):
            if gargoyle.is_active(gargoyle_switch):
                return func(*args, **kwargs)
            return decorator(func)(*args, **kwargs)
        return _inner_wrapper
    return _outer_wrapper

Usage:

@toggle_decorator('gargoyle_switch_name', decorator)
def foo(): ...

This will let you enable the switch to disable the decorator[1]. If this is something you want, I'm happy to look into gargoyle code and prepare a PR.

[1] This is a policy we're keeping so we don't have to toggle switches when releasing a new feature. It is of course easy to revert this, which I believe fits better in general gargoyle usage.

adamchainz commented 6 years ago

This is a nice snippet but I'm not sure how generally applicable it is. If you wrote the decorator, why not put the if gargoyle.is_active check inside the decorator itself?

Timvde commented 6 years ago

The decorator is coming from a third party library, so we can't change it without forking the library itself. I'm also not sure useful this is in general, which is why I created an issue with the snippet instead of doing the whole forking/cleaning up code/writing tests dance to create a proper PR before I had confirmation that it'll be accepted.

Anyway, I'm fine with it if you don't deem it useful enough to upstream, at least it will get indexed by search engines for people to find :)

adamchainz commented 6 years ago

Thanks for more explanation. Yeah I think it's not general enough to merge, but this will let people find it if they need it. Cheers!