django-wiki / django-nyt

Notification system for Django with batteries included: Email digests, user settings, JSON API
Apache License 2.0
144 stars 47 forks source link

Testable settings pattern #124

Closed benjaoming closed 10 months ago

benjaoming commented 10 months ago

As I've just discovered this, I wanna make sure to note it somewhere:

The pattern that we have for settings in django-nyt (and django-wiki) isn't perfect because it doesn't support the @override_settings decorator used in testing.

There are some suggestions in this Stack Overflow post: https://stackoverflow.com/questions/8428556/django-default-settings-convention-for-pluggable-app

I haven't quite understood which is the perfect pattern yet. One of the reasons to have a local settings module would be to easily gather an overview and at the same time document settings. Let's see if we can keep this.

benjaoming commented 10 months ago

It seems that Python 3.7+ can do __getattr__ on modules so we can resolve things in a lazy way... gonna have a look at how we can both define our defaults and resolve Django settings from their current value. See: https://stackoverflow.com/questions/1462986/lazy-module-variables-can-it-be-done

It'd be nice if this can be used to easily ship documentation, too.

oscarmcm commented 10 months ago

What do you mean by:

It'd be nice if this can be used to easily ship documentation, too.

benjaoming commented 10 months ago

Glad you asked! :) Here's a disproportionately ambitious and overthought answer...

I've heard a lot of people mention "locality of behavior". On this topic, I find it important to ensure that configuration variables are documented in the code, where they are defined. Such that the documentation and definition isn't allowed to deviate. Currently, our documentation section about settings variables is able to pull in a somewhat meaningful list of settings variables:

https://github.com/django-wiki/django-nyt/blob/6fae9cf8579a905f93b1af55f834872b41b1e393/docs/configuration.rst?plain=1#L1-L13

(I say "somewhat" because of the sentence at the top where it's mentioned that you need to prefix everything with NYT_) at the beginning.

In order to keep being able to have good API docs for configuration, I think it's important to avoid manually maintained .rst files full of settings variables an descriptions. I'm not sure that it can be done.

I had a look at some other projects with many settings. Many of them maintain manually written .rst documentation for all their Django settings, even the ones with a lot of them. But I found that Celery has an interesting Sphinx directive settings: https://github.com/celery/celery/blob/b8c67a7a9cc1dfd30b292b4cac955bc8bf7e703f/docs/userguide/configuration.rst?plain=1