Skip to content

[AAP-34936] Manage Django Settings with Dynaconf #15702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

rochacbruno
Copy link
Member

@rochacbruno rochacbruno commented Dec 11, 2024

AAP-34936

New or Enhanced Feature

Settings now managed by Dynaconf.

export DJANGO_SETTINGS_MODULE=awx.settings
dynaconf list
dynaconf inspect

NOTE: Currently depends on DAB ansible/django-ansible-base#689 to be merged.

DOCS https://github.com/ansible/django-ansible-base/blob/devel/docs/lib/dynamic_config.md

and https://gist.github.com/rochacbruno/3779c7a6acf341b22ea7c26c9f4d023e

@github-actions github-actions bot added component:api dependencies Pull requests that update a dependency file labels Dec 11, 2024
Copy link

@AlanCoding
Copy link
Member

So how would the awx/conf (dynamic database backed settings) be handled with this?

Copy link

codecov bot commented Feb 10, 2025

Codecov Report

Attention: Patch coverage is 63.91753% with 35 lines in your changes missing coverage. Please review.

Project coverage is 75.27%. Comparing base (ee739b5) to head (cdabeb6).
Report is 70 commits behind head on devel.

✅ All tests successful. No failed tests found.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rochacbruno rochacbruno changed the title P.O.C: Manage Django Settings with Dynaconf [AAP-34936] Manage Django Settings with Dynaconf Feb 10, 2025
@rochacbruno rochacbruno marked this pull request as ready for review February 10, 2025 19:47
@rochacbruno
Copy link
Member Author

So how would the awx/conf (dynamic database backed settings) be handled with this?

@AlanCoding will not, this is going to be on phase 2 only.

@rochacbruno rochacbruno force-pushed the dynaconf branch 2 times, most recently from 0981fdf to 4a6f6e8 Compare February 17, 2025 11:22
@rooftopcellist
Copy link
Member

FYI, it looks like the awx-operator CI check failed. The AWX deployment by the operator itself was successful, but the job template failed. This may be related to the settings changes in this PR. I would next check that job templates run successfully in the AWX dev environment. If they do, then there are likely awx-operator changes needed around how we mount-in the settings.py file into the task and web pods.

@rochacbruno rochacbruno force-pushed the dynaconf branch 2 times, most recently from d482a87 to 0eb7b4e Compare February 20, 2025 19:41
@AlanCoding

This comment was marked as resolved.

@AlanCoding

This comment was marked as resolved.

@rochacbruno
Copy link
Member Author

@AlanCoding the error above is a validation that I explicitly added to replicate the following lines

# Attempt to load settings from /etc/tower/settings.py first, followed by
# /etc/tower/conf.d/*.py.
try:
include(settings_file, optional(settings_files), scope=locals())
except ImportError:
traceback.print_exc()
sys.exit(1)
except IOError:
from django.core.exceptions import ImproperlyConfigured
included_file = locals().get('__included_file__', '')
if not included_file or included_file == settings_file:
# The import doesn't always give permission denied, so try to open the
# settings file directly.
try:
e = None
open(settings_file)
except IOError:
pass
if e and e.errno == errno.EACCES:
SECRET_KEY = 'permission-denied'
LOGGING = {}
else:
msg = 'No AWX configuration found at %s.' % settings_file
msg += '\nDefine the AWX_SETTINGS_FILE environment variable to '
msg += 'specify an alternate path.'
raise ImproperlyConfigured(msg)
else:
raise

My logic was: If neither AWX_SETTINGS_FILE or any file from /etc/tower or /etc/ansible-* is found, it raises an error, because at least one config file is required.

So it look like that is not the case right?

@AlanCoding
Copy link
Member

The expectation is that /etc/tower/settings.py exists in the context I'm pasting that from.

My logic was: If neither AWX_SETTINGS_FILE or any file from /etc/tower or /etc/ansible-* is found, it raises an error, because at least one config file is required.

Help me talk that out a little better. Again, I can wave away corner cases here and presume that settings_file is /etc/tower/settings.py. So what is this doing?

    if not included_file or included_file == settings_file:

Specifically, included_file == settings_file. In the happy path of the really default installation, I expect this is always true. Maybe I'm barking up the wrong tree here, but I'm not sure.

@rochacbruno
Copy link
Member Author

@AlanCoding this is logic from split_settings

included_file = locals().get('__included_file__', '') 
     if not included_file or included_file == settings_file: 

From what I learned, split_settings keeps state of which files already loaded in the __included_file__ variable.

As I am removing split_settings, this logic is not applying anymore, as dynaconf tracks loaded files in a different way.

@AlanCoding
Copy link
Member

Hey, thanks. I saw some pushes and re-tested. From that, I can see this is making progress. Before, it failed on an awx-manage command. Now it makes it past that and gives a 500 error from a web request. This is still later in the process, so it's getting closer to functionality. The server logs from that 500 error are:


2025-02-25 14:31:05,991 ERROR    [3c5320affe93491a8857aa2043ad7c9b] django.request Internal Server Error: /api/v2/ping/
Traceback (most recent call last):
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/django/utils/deprecation.py", line 133, in __call__
    response = self.process_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/main/middleware.py", line 50, in process_request
    if settings.AWX_REQUEST_PROFILE:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 543, in __getattr_without_cache__
    return getattr(self._wrapped, name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 441, in __getattr__
    value = self._get_local_with_cache(name)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/cachetools/__init__.py", line 838, in wrapper
    v = method(self, *args, **kwargs)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 432, in _get_local_with_cache
    return self._get_local(name)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 351, in _get_local
    self._preload_cache()
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 298, in _preload_cache
    defaults_snapshot = self._get_default('DEFAULTS_SNAPSHOT')
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/awx/conf/settings.py", line 418, in _get_default
    return getattr(self.default_settings, name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Settings' object has no attribute 'DEFAULTS_SNAPSHOT'

Here, I must apologize for some tech-debt on the AWX side. The DEFAULTS_SNAPSHOT is used by config-tower-in-tower (database settings). This takes the local settings (assuming they are static content) and then freezes them in a dictionary (which it then saves into locals). This is then used to know if a submitted value matches the value defined in file-based settings or not. That allows a PATCH to /api/settings/all/ for example, filter out the values which are the same as defaults. I'm guessing the reason the stack trace above references it is because it doesn't want to put settings value in the cache if they are at the default value anyway.

@jessicamack
Copy link
Member

FYI, it looks like the awx-operator CI check failed. The AWX deployment by the operator itself was successful, but the job template failed. This may be related to the settings changes in this PR. I would next check that job templates run successfully in the AWX dev environment. If they do, then there are likely awx-operator changes needed around how we mount-in the settings.py file into the task and web pods.

@rooftopcellist re-running the awx-operator job to confirm the CI check is still not passing but building the dev environment off of the latest commit 5df4c0b, I am able to run the Demo Job Template successfully.

@rochacbruno
Copy link
Member Author

@jessicamack I am running the dev environment locally, what are the steps for me to run the demo job template?

@jessicamack
Copy link
Member

jessicamack commented Feb 25, 2025

@rochacbruno do you have the UI built? you should be able to view it on port 8043. if not you might need to run awx-manage collectstatic first. from the UI home you can click Jobs and you'll see a Demo Job Template listed

@rochacbruno
Copy link
Member Author

@jessicamack

I executed the collectstatic inside the container and then restarted but I see "Oops... Looks like the UI wasn't properly built"

@jessicamack
Copy link
Member

@rochacbruno if you run make ui what do you get?

@rochacbruno
Copy link
Member Author

Thanks @jessicamack it worked and Demo Job Template runs without problems on my local environment.

@AlanCoding on the dev env, the DEFAULTS_SNAPSHOT is there and I compared with the keys present when running from devel branch and it is the same.

Not sure why you are getting error that the key is not defined.

@rochacbruno
Copy link
Member Author

Thanks @jessicamack @AlanCoding now it is working!

api-schema test is failing but it seems to be also failing on other PRs

@rochacbruno rochacbruno force-pushed the dynaconf branch 4 times, most recently from 7e8ad3c to b596859 Compare March 6, 2025 17:08
@rochacbruno rochacbruno requested a review from AlanCoding March 6, 2025 17:11
@AlanCoding
Copy link
Member

I'm unclear on this failure

  File "/awx_devel/awx/main/tests/functional/models/test_events.py", line 170, in test_host_metrics_update
    assert len(HostMetric.objects.filter(Q(deleted=False) & Q(deleted_counter=1) & Q(last_deleted__isnull=False))) == 5
AssertionError: assert 6 == 5
 +  where 6 = len(<QuerySet [<HostMetric: HostMetric object (1)>, <HostMetric: HostMetric object (3)>, <HostMetric: HostMetric object (5)>, <HostMetric: HostMetric object (7)>, <HostMetric: HostMetric object (9)>, <HostMetric: HostMetric object (11)>]>)
 +    where <QuerySet [<HostMetric: HostMetric object (1)>, <HostMetric: HostMetric object (3)>, <HostMetric: HostMetric object (5)>, <HostMetric: HostMetric object (7)>, <HostMetric: HostMetric object (9)>, <HostMetric: HostMetric object (11)>]> = filter(((<Q: (AND: ('deleted', False))> & <Q: (AND: ('deleted_counter', 1))>) & <Q: (AND: ('last_deleted__isnull', False))>))
 +      where filter = <django.db.models.manager.Manager object at 0x7f57e14d6dd0>.filter
 +        where <django.db.models.manager.Manager object at 0x7f57e14d6dd0> = HostMetric.objects
 +      and   <Q: (AND: ('deleted', False))> = Q(deleted=False)
 +      and   <Q: (AND: ('deleted_counter', 1))> = Q(deleted_counter=1)
 +      and   <Q: (AND: ('last_deleted__isnull', False))> = Q(last_deleted__isnull=False)

I'll restart it in case it's flaky

@AlanCoding
Copy link
Member

We have identified the source of the failure for api-tests, and hope to have it fixed soon. Not an issue with this branch.

@AlanCoding
Copy link
Member

Fix to that was merged in #15875, it should clear up after rebase

Dynaconf is being added from DAB factory to load Django Settings
Copy link

sonarqubecloud bot commented Mar 6, 2025

@AlanCoding AlanCoding merged commit 0ffe04e into ansible:devel Mar 7, 2025
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants