Create CallableSetting

[stinovlas]

In several of my projects, I needed to create callable setting, i.e. string setting that would contain dotted path to a callable. There is one example of how I implemented it:

import appsettings
from django.utils import module_loading

class CallableSetting(appsettings.StringSetting):
    """
    Callable setting.

    Contains dotted path refering to callable.
    """

    def transform(self, value):
        """Translate dotted path to callable."""
        return module_loading.import_string(value)

    def checker(self, name, value):
        """Check whether dotted path refers to callable."""
        transformed_value = self.transform(value)
        if not callable(transformed_value):
            raise ValueError('{} must be a dotted path to callable'.format(name))

I just noticed that ObjectSetting can contain path to a callable as well. However, it does not have custom validation checking whether path leads to a callable.

Also, ObjectSetting uses its own import machinery, that could might be replaced by django.utils.module_loading.import_string (however, the functionality is a little bit different, so it might be a breaking change).

I propose creating CallableSetting as a subclass of ObjectSetting, containing specific check whether the value points to a callable. If you agree, I will be happy to submit a pull request.

We can also discuss replacing ObjectSetting.transform by django.utils.module_loading.import_string (possibly a breaking change).

[pawamoy]

Still using checkers 😉? They were deprecated in a previous PR introducing Django-style validators. Unfortunately, for this use-case a validator would not be enough, since they operate on the raw value and not the transformed one.

I think if we generalize the use-case, we would need to add a way to perform validation on the transformed value. I'd like your opinion on this, as well as @ziima's.

About django.utils.module_loading.import_string, I think it's doing less than the code we have in ObjectSetting.transform: import_string seems to only support one level of nesting inside the final module.

For example, with:

• a/b/c.py
• class C: D = 1 in c.py
• object setting is a.b.c.C.D

...then import_string would fail where the current code would not. Though I did not verified this. Just based on the source here. Also, not sure if this particular feature of ObjectSetting is actually used by someone, or if this is even useful. But yeah I guess it would be a breaking change.

[stinovlas]

About django.utils.module_loading.import_string, I think it's doing less than the code we have in ObjectSetting.transform: import_string seems to only support one level of nesting inside the final module.

For example, with:
* a/b/c.py
* class C: D = 1 in c.py
* object setting is a.b.c.C.D

...then import_string would fail where the current code would not. Though I did not verified this. Just based on the source here. Also, not sure if this particular feature of ObjectSetting is actually used by someone, or if this is even useful. But yeah I guess it would be a breaking change.

Yeah, that's what I thought as well. The current code in Django 2.2 is a little bit different, but the property you describe still stands.

I have never used ObjectSetting for anything that import_string couldn't handle, but I guess that it could be a legitimate use case, although somewhat obscure.

[pawamoy]

I usually imagine someone using classes as enums, like I did here in another project:

# callbacks.py

class STATE:
    class SEARCH:
        PATTERN, SELECT = range(2)
    class GRANT:
        USER, PRIVILEGE = range(2)
    REVOKE = GRANT

The setting could then be APP_SEARCH_STATES = "callbacks.STATE.SEARCH".

Anyway, I agree that using a Django battle-tested function could be better than using custom code, but if the main concern is avoiding duplicated code (when adding a new CallableSetting), then we can also move the few lines in a utility function called import_object or similar 😄.

---

To go back to the CallableSetting. We could do this indeed:

class CallableSetting(ObjectSetting):
    def validate(self, value):
        transformed_value = self.transform(value)
        if not callable(transformed_value):
            raise ValueError('{} must be a dotted path to callable'.format(self.full_name))

But now if the user wants to add even more custom validation, they will have to call super().validate(value). Maybe it's not an expensive price to pay after all.

[stinovlas]

I usually imagine someone using classes as enums, like I did here in another project:

That's actually pretty good use case, thank you! I guess we can keep it the way it is.

To go back to the CallableSetting. We could do this indeed:

class CallableSetting(ObjectSetting):
    def validate(self, value):
        transformed_value = self.transform(value)
        if not callable(transformed_value):
            raise ValueError('{} must be a dotted path to callable'.format(self.full_name))

But now if the user wants to add even more custom validation, they will have to call super().validate(value). Maybe it's not an expensive price to pay after all.

I think that calling super().validate(value) is not obtrusive at all. CallableSetting seems to work pretty well as you designed it. Should I create a pull request?

[ziima]

And do we actually want to run validation on a raw value instead of the final one? Django objects (models, forms) validate final values, not the raw ones.

[stinovlas]

And do we actually want to run validation on a raw value instead of the final one? Django objects (models, forms) validate final values, not the raw ones.

Yeah, that might be even better solution.

[pawamoy]

Interesting point.

Let say we validate only once transformed. For settings which are not transformed, the behavior stays the same.

For the ones which are transformed, the first thing I dislike is that we will run the transform function at least twice: once to check all settings when starting up the project, a second time when accessing the setting (from an instance of AppSettings).

Usually the transformation will not be heavy, but if it is, it will slow down the startup (maybe I'm a bit too paranoid about perf though).

Maybe I'm going too far but we also have to consider it's possible that the transformation has side-effects, which could cause issues if ran twice or more. I'm getting a bit crazy, but let say you have a setting, which when transformed is creating a file or launching a program listening to a port or something, and then return the file path or the PID of the program. It could pass the initial check, and fail when accessing the setting the second time.

A solution to this double transformation would be to cache the transformed value inside of the setting itself, as long as the raw value is the same (self._cache_raw and self._cache_transformed, the latter being invalidated when the former is not equal to self.raw_value anymore).

Also maybe sometimes you really want to validate the raw value and not the transformed one, because if the raw value is OK, you know the transformed one will be too (you are the one writing the transform function after all).

So the question is:

• do we need pre-transform validation? (I think yes)
• do we need post-transform validation? (it's a yes too for @stinovlas' use-case)
• do we need both? (yes + yes = yes)

Then how should this be implemented?
Two ways I can think of:

• adding of post_transform_validators or post_validators parameter to the Setting class.
• derivate validators into two classes, PreValidator and PostValidator, and use the raw value or transformed one based on a condition in the setting check method: if isinstance(validator, PreValidator): ... elif isinstance(validator, PostValidator): ...

Sorry for the long text and waiting for your thoughts on this 🙂

[stinovlas]

Well, if we'll have a transformation cache, we can just call self.transform(value) in the validator. I don't think there is any harm in that. Separating pre-transform and post-transform validators seems like an overkill.

Are you concerned about running some validators before the transform is called for the first time?

[pawamoy]

Yeah okay. Let's create a CallablePathSetting inheriting from ObjectSetting with a custom validate method that checks that the result of self.transform is a callable.

I think we should name it CallablePathSetting because with CallableSetting, one would expect to pass a callable directly, not a dotted path (btw ObjectSetting is confusing in the same way, maybe we should change it). You can submit a PR if you wish! Otherwise tell me and I'll do it.

For posterity: I thought of a third way for pre/post validation: by adding a parameter to the validators constructor, something like at=Validator.PRE_TRANSFORM, with another choice of course: Validator.POST_TRANSFORM. Whatever, just a note to myself 😉

[stinovlas]

I should get to the PR before the end of this week 😉.