ansible / django-ansible-base

Apache License 2.0
12 stars 43 forks source link

Save simple default settings in a separate module #526

Closed AlanCoding closed 1 month ago

AlanCoding commented 1 month ago

Working through https://github.com/ansible/galaxy_ng/pull/2202 with @jctanner I discovered it wasn't loading the settings. In short, I don't think we can rely on accurately having INSTALLED_APPS populated at the time we needed to load the defaults.

But we shouldn't need this. If an app wants to bypass our recommended system (which it is already doing), then it should have a constant data structure to import.

I wanted to do this with dictionaries too, but it seems we have special cases for each entry, which... I can't tough. So this is unfortunately incomplete.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

AlanCoding commented 1 month ago

i.e. What exactly is the problem with INSTALLED_APPS?

I'll link you the thing that ultimately needs to be fixed here

https://github.com/ansible/galaxy_ng/blob/b26816eaf9c1e34f4033835e09125013a5d182d3/galaxy_ng/app/dynaconf_hooks.py#L732

There's a method that needs to return a dictionary of the settings from DAB. It's probably already obvious to you that this poses an issue for any settings that have to get fine-grained merged. But I don't think galaxy_ng is using any part of DAB that requires that at this moment, and I'm not sure how it would work if we did have that situation.

But you mention INSTALLED_APPS. This would have to be included in local variables to pass the value, and that's just not the right way to do thing. I don't even really know if split_settings works within a function's variable scope, or if it might mess up something else.

AlanCoding commented 1 month ago

For an immediate solution I prefer the other approach. We could do a version of this later (to define truly static settings), but it is not necessary.