Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Unexpected SystemSettings interaction causing failure to run via wsgi.py #11660

Open
jacobtylerwalls opened this issue Nov 27, 2024 · 3 comments · May be fixed by #11687
Open

Unexpected SystemSettings interaction causing failure to run via wsgi.py #11660

jacobtylerwalls opened this issue Nov 27, 2024 · 3 comments · May be fixed by #11687
Assignees
@jacobtylerwalls
Labels
Priority: High Type: Bug Something isn't working

Comments

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Nov 27, 2024 •
edited
Loading

@ekansa reported on the forum a failure to run an arches project via wsgi.py, i.e. not run with runserver. The TL;DR is that we have the default value of APP_NAME being stored in a system settings tile and overriding the project settings file, only under wsgi.py.

Traced it to this unexpectedly evaluating True:

arches/arches/urls.py

Line 789 in f3390ef

if settings.APP_NAME == "Arches":

... because wsgi.py loads system settings from the database:

arches/arches/wsgi.py

Line 39 in f3390ef

settings.update_from_db()

... and we save the value "Arches" for APP_NAME in the database during setup_db, which is going to be incorrect for all cases (only applies to running "naked" core arches like a project):

arches/arches/db/system_settings/Arches_System_Settings.json

Lines 153 to 156 in f3390ef

"en":{
"direction":"ltr",
"value":"Arches"
}

arches/arches/management/commands/setup_db.py

Lines 234 to 236 in f3390ef

management.call_command(
"packages", operation="import_graphs", source=settings_graph
)

@chrabyrd and I didn't notice this when developing f569e09 and related features because we were running the devserver, but arguably this is an orthogonal bug.

We have other cases where we need the value of APP_NAME to be correct:

arches/arches/settings_utils.py

Lines 161 to 162 in f3390ef

if settings.APP_NAME == "Arches":
base_path = root_dir_path

I reproduced on 7.6.3 with Eric's instructions:

  • clone opencontext/arches-via-docker
  • checkout the 'local' branch
  • cp edit_dot_env .env
  • make DEBUG=False (in .env) and also change the SHOW_LANGUAGE_SWITCH to be True
  • commented out DEPLOY_HOST in settings_local since there wasn't a fallback
  • prepend npm i [email protected] to the build development command to work around vue-tsc doesn't work with typescript 5.7.2 vuejs/language-tools#5018. (This will hopefully disappear in a few days.)
  • visit the server at 0.0.0.0:8004

Next steps

A workaround is to comment out the last line of wsgi.py that calls update_from_db(), or updating the invalid tile value in pg directly. But regardless of the wisdom of that, there a few things to look into, some of which shouldn't be done in a patch?

  • fix the values in these tiles (migration? in v8?)
  • update the python SystemSettings to ignore this node (can do in a patch)
  • stop creating values in these tiles (in v8?)

cc/ @aarongundel
@jacobtylerwalls jacobtylerwalls added Type: Bug Something isn't working Priority: High labels Nov 27, 2024
@jacobtylerwalls jacobtylerwalls changed the title Unexpected system_settings interaction causing failure to run WSGI application Unexpected SystemSettings interaction causing failure to run WSGI application Nov 27, 2024
@chiatt chiatt added this to pipeline Nov 27, 2024
@jacobtylerwalls jacobtylerwalls changed the title Unexpected SystemSettings interaction causing failure to run WSGI application Unexpected SystemSettings interaction causing failure to run via wsgi.py Nov 27, 2024
@ekansa
Copy link

ekansa commented Nov 27, 2024 •
edited
Loading

@jacobtylerwalls thanks for the deep investigation and diagnosis!

@jacobtylerwalls
Copy link
Member Author

Throwing out ideas for better heuristics for "is the currently running Django project core arches itself?":

this should only evaluate True for running core arches, but it's a little fiddly:

>>> from pathlib import Path
>>> from django.conf import settings
>>> Path(settings.APP_ROOT).parent == Path(settings.ROOT_DIR)
True

@jacobtylerwalls
Copy link
Member Author

Also worth looking into whether core urls.py can just do the same check that's working well in projects?

if settings.ROOT_URLCONF == __name__:

@jacobtylerwalls jacobtylerwalls self-assigned this Dec 6, 2024
@jacobtylerwalls jacobtylerwalls moved this to 🏗 In Progress in pipeline Dec 13, 2024
jacobtylerwalls added a commit that referenced this issue Dec 13, 2024
@jacobtylerwalls
Use same guard for "last" project as project-level urls.py #11660
a7c5c85 
This check sniffs for whether this urls.py is the one being run as a project
(not included by any further apps). For core arches, this would only be true if running core arches without a project. This was failing on a Django exception
inside i18n_patterns().

This check is the same check that we're using in project-level settings.py
jacobtylerwalls added a commit that referenced this issue Dec 13, 2024
@jacobtylerwalls
Update system settings from db in development also #11660
3658a2c
jacobtylerwalls added a commit that referenced this issue Dec 13, 2024
@jacobtylerwalls
Add changelog re #11660
d49ae58
jacobtylerwalls added a commit that referenced this issue Dec 13, 2024
@jacobtylerwalls
Use same guard for "last" project as project-level urls.py #11660
821e901 
This check sniffs for whether this urls.py is the one being run as a project
(not included by any further apps). For core arches, this would only be true if running core arches without a project. This was failing on a Django exception
inside i18n_patterns().

This check is the same check that we're using in project-level settings.py
jacobtylerwalls added a commit that referenced this issue Dec 13, 2024
@jacobtylerwalls
Update system settings from db in development also #11660
fbc966a
@jacobtylerwalls jacobtylerwalls linked a pull request Dec 13, 2024 that will close this issue
Open
11 tasks
@jacobtylerwalls jacobtylerwalls linked a pull request Dec 13, 2024 that will close this issue
Fix enabling multiple languages if the APP_NAME in the db was "Arches" #11660 #11687
Open
11 tasks
@jacobtylerwalls jacobtylerwalls moved this from 🏗 In Progress to 👀 In Review in pipeline Dec 13, 2024
jacobtylerwalls added a commit that referenced this issue Dec 14, 2024
@jacobtylerwalls
Revert "Update system settings from db in development also #11660"
cbd60c1 
This reverts commit fbc966a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jacobtylerwalls jacobtylerwalls

Labels
Priority: High Type: Bug Something isn't working
Projects
pipeline
Status: 👀 In Review
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix enabling multiple languages if the APP_NAME in the db was "Arches" #11660 archesproject/arches
2 participants
@ekansa @jacobtylerwalls