Readthedocs.org: Move our application settings to a READTHEDOCS prefix on our settings

[agjohnson]

We have a number of settings that don't have a READTHEDOCS prefix and this makes our settings hard to find. The core team would like to consolidate our application settings to use a prefix so that they are easier to find and search for in our code.

This is related to how we would like to remove the use of getattr usage in #2140. It's probably either blocked or at least related.

[davidfischer]

This is related to how we would like to remove the use of getattr usage in #2140. It's probably either blocked or at least related.

Let's wait on this one at least until #5588 is merged. That should be soon though.

[saadmk11]

@davidfischer I would like to work on this after #5588 gets merged but I'm not quite sure what @agjohnson means by application settings. which settings from the settings file should have the Prefix? Can you please explain?

[stsewd]

@saadmk11 all the settings that we have in settings/base.py and that we use for internal usage only (like DOCKER_ENABLE), should be renamed to RTD_DOCKER_ENABLE or READTHEDOCS_DOCKER_ENABLE (first one is shorter and clear for me)

[saadmk11]

@stsewd Thanks a lot for the clarification :)

[rshrc]

@stsewd I was working on this issue, working with settings for the first time and I was unable to understand whether we should make changes to the CSRF and Database and API hitting settings.. refactoring them with the RTD_ prefix broke a lot of things which gave rise to errors on running the development server like AttributeError: 'Settings' object has no attribute 'DOCKER_IMAGE_SETTINGS'. How to do I get around this?

[davidfischer]

I think every setting that we define should be prefixed with RTD_. Every setting that either comes from Django or a 3rd party package shouldn't change. The DOCKER_IMAGE_SETTINGS for example are defined by us.

[rshrc]

I think every setting that we define should be prefixed with RTD_. Every setting that either comes from Django or a 3rd party package shouldn't change. The DOCKER_IMAGE_SETTINGS for example are defined by us.

Cool then, I'll segregate things as such then.

[Iamshankhadeep]

I see no one submitted PR on this issue can I work on it?

[stsewd]

@Iamshankhadeep sure!

[Iamshankhadeep]

@stsewd work done, review it please.

[stsewd]

There are still more settings to rename

[Iamshankhadeep]

Do I need to change the name for all setting?

[stsewd]

@Iamshankhadeep yes, but please make separate PRs for each setting so the changes are easy to review.

[ericholscher]

Let's please not do a large renaming of our settings in a random way. We really need to make sure we handle these right, and deprecate them properly. Just renaming things randomly is going to break a ton of downstream stuff for no real value.

[agjohnson]

I brought up in slack that we should throw deprecation warnings for any of the old settings. We can do this with django's system checks probably. Deprecated settings should be well documented as well, otherwise it's very confusing what settings have changed.

[Lokesh2703]

@stsewd Can I do renaming for Path Variables in settings/base.py

[stsewd]

@Lokesh2703 what path variables? If you are going to rename a setting, make sure to have a way to show they are deprecated.