I ran into an error that occured due to a typo in the name of a waffle flag in my code. It would have been much better (IMO), and the bug would have been picked up much earlier, had flag_is_active failed hard for non-existent flags. Failing silently can be dangerous, and whilst I appreciate I can set WAFFLE_LOG_MISSING_FLAGS to logging.ERROR to increase visibility, this will still not cause a request to fail hard. In development environments, I tend not to pay particular attention to logs unless I'm specifically looking for something. However, a hard failure would pick up the typo before it makes its way to production.
Is this configuration bloat??
Obviously, not everyone would want the behaviour I suggest above (quite apart from the fact that it would be a breaking change). As such, an extra configuration setting such as WAFFLE_RAISE_MISSING_FLAGS would be needed. There are currently quite a lot of configuration settings for waffle. For each of FLAG, SWITCH and SAMPLE we have:
WAFFLE_*_DEFAULT
WAFFLE_CREATE_MISSING_*S
WAFFLE_LOG_MISSING_*S
With this in mind, we probably do not want an extra configuration setting. There are no doubt other behaviours that other uses would wish for in the case of a missing flag, and it's untenable to provide a new setting for each of them.
A proposed solution
Instead of introducing a new config setting, I propose that we replace WAFFLE_CREATE_MISSING_*S and WAFFLE_LOG_MISSING_*S with a new setting WAFFLE_HANDLE_MISSING_*S which would be a string that points to a function that would be called when an entity is missing. For example:
# settings.py
WAFFLE_HANDLE_MISSING_FLAGS = "some_file.handle_missing_flags"
# some_file.py
def handle_missing_flags(name):
# potentially log a message, create a flag and do anything else you want
# optionally return a flag
(If a flag is returned the flag would be cached, and the value from the flag would be used, otherwise the default value would be used - as is the current behaviour).
This would have several advantages.
The default function would use the existing WAFFLE_CREATE_MISSING_*S and WAFFLE_LOG_MISSING_*S settings so that introducing the setting would result in no change in behaviour.
We can then utilise the django checks framework to provide warnings that the old settings will eventually be deprecated, and in time update those warnings to errors. Suitable documentation could also be provided to guide migration to the new system (although this should be reasonably straight-forward). This will mean a pleasant deprecation timeline.
This allows users greater flexibility to provide the individual functionality they want.
This actually reduces the number of configuration settings whilst protecting against future settings bloat.
(Part of me would like to subsume WAFFLE_*_DEFAULT into the new setting too, but seeing as the default values are used in WaffleJS this would be difficult.)
If there is support for this proposition I would be more than happy to submit a PR. I look forward to any feedback on the above.
The problem
I ran into an error that occured due to a typo in the name of a waffle flag in my code. It would have been much better (IMO), and the bug would have been picked up much earlier, had
flag_is_activefailed hard for non-existent flags. Failing silently can be dangerous, and whilst I appreciate I can setWAFFLE_LOG_MISSING_FLAGStologging.ERRORto increase visibility, this will still not cause a request to fail hard. In development environments, I tend not to pay particular attention to logs unless I'm specifically looking for something. However, a hard failure would pick up the typo before it makes its way to production.Is this configuration bloat??
Obviously, not everyone would want the behaviour I suggest above (quite apart from the fact that it would be a breaking change). As such, an extra configuration setting such as
WAFFLE_RAISE_MISSING_FLAGSwould be needed. There are currently quite a lot of configuration settings for waffle. For each ofFLAG,SWITCHandSAMPLEwe have:WAFFLE_*_DEFAULTWAFFLE_CREATE_MISSING_*SWAFFLE_LOG_MISSING_*SWith this in mind, we probably do not want an extra configuration setting. There are no doubt other behaviours that other uses would wish for in the case of a missing flag, and it's untenable to provide a new setting for each of them.
A proposed solution
Instead of introducing a new config setting, I propose that we replace
WAFFLE_CREATE_MISSING_*SandWAFFLE_LOG_MISSING_*Swith a new settingWAFFLE_HANDLE_MISSING_*Swhich would be a string that points to a function that would be called when an entity is missing. For example:(If a flag is returned the flag would be cached, and the value from the flag would be used, otherwise the default value would be used - as is the current behaviour).
This would have several advantages.
WAFFLE_CREATE_MISSING_*SandWAFFLE_LOG_MISSING_*Ssettings so that introducing the setting would result in no change in behaviour.(Part of me would like to subsume
WAFFLE_*_DEFAULTinto the new setting too, but seeing as the default values are used in WaffleJS this would be difficult.)If there is support for this proposition I would be more than happy to submit a PR. I look forward to any feedback on the above.