Argus
Argus copied to clipboard
Add settings toggle for local frontend
May be replaced with something else that depends on #904
Adds a setting to turn the new frontend on and the old frontend off. It is not possible to run both at the same time due to some hackery the old frontend needs for OAuth2.
Previously, having a setting in argus.site.settings.base that toggled between old and new frontend via lots of code and a new setting:
elfjes: Hmm I don't think I like this. Without having thorouhly examined the implications of this PR it feels like it makes Argus more opinionated in how it wants to be used/configured. For example, we're not using the apps plugin system, which means that we're currently fully outside the flow of EXTRA_APPS and OVERRIDING_APPS. We have our own way of including argus_htmx. It feels like this PR wants to nudge us to a different method, and I fear we'll lose the flexibility we have now. We'd maybe want to set FRONTEND_APP to None to not have Argus include argus_htmx itself, but then Argus assumes we run the old fontend/api-only and that's also not what we want.
Like I said, I haven't looked at the implications thoroughly, but this is my initial impression.
I'd rather see Argus creating a new settings files that can be pointed to in prod that sets up argus for htmx usage rather than to include everything in
site/settings/base.pyandsite/urls.,py, and then we can happily not do anything with those files :)pehaps the oauth functionality can become an app in and of itself than can be included/excluded at leisure
hmpf: Yeah I realized the oauth stuff can be split out into a different PR, BUT: the current, old-frontend oauth2 stuff is hardcoded to the vendored psa backend because of the loginwrapper. So in order to use ANY oauth2 we need to know that we are NOT using the old frontend, so we need a switch for that. FRONTEND_URL the setting is also used everywhere and with kubernetes it makes it easier to have the setting because django struggles to find what host it publicly runs on otherwise. (If we log with hostname from kubernetes we do not get a hostname but the node/pod-id. Much useful, very quality.)
hmpf: I thought this way of pulling in all the settings the frontend needs, while still supporting the old frontend, was quite neat. Every setting that argus_htmx must have is in argus_htmx.appconfig, and every setting can be overruled if necessary/as usual in a localsettings file.
This is still compatible with (OVERRIDING|EXTRA)_APPS, and worst case it is always possible to clone base.py completely. The trick is, again, the site urls.py. But what root urls.py to use is still a setting.
elfjes: Oh I'm all for factoring out the hardcoded oauth stuff into a separate thing (eg app). My concern is with settings/base and site/urls. I'd much rather see that being more composable rather than behaviour basecd on toggles. That way we can each compose our own settings/urls as we see fit. IMHO Argus should become less opinionated on how it's used/integrated, not more.
I think that requiring cloning base.py should only be done as a last resort, since we want to minimize code duplication.
Codecov Report
Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
Project coverage is 83.36%. Comparing base (
ce09d7e) to head (ca607aa). Report is 1 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/argus/htmx/htmx_urls.py | 0.00% | 5 Missing :warning: |
| src/argus/htmx/settings.py | 0.00% | 5 Missing :warning: |
| src/argus/htmx/urls.py | 0.00% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #899 +/- ##
==========================================
- Coverage 83.63% 83.36% -0.27%
==========================================
Files 95 98 +3
Lines 4137 4149 +12
==========================================
- Hits 3460 3459 -1
- Misses 677 690 +13
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Question, if we'd want to use OVERRIDING_APPS / EXTRA_APPS. we'd want to do this without environment variables but add the relevant structs directly in our settings.py. Right now the only way I can see that we could do that is by calling
update_settingsourselves, right?
update_settings needs to be run in the final, outermost settings-file yes. Remember globals(). No env-vars needed.
update_settings now takes the boolean flag override (default: False).
It would be nice if we could do something like this:
geant_settings.py
from argus_htmx import app_settings update_settings(app_settings)That way we can have the magic, while still being in control of it :)
Just copy the stuff from argus.htmx.settings into the geant settings, then override what you need. If you have your own geant app_settings you could run update_settings twice.
Either make an instance of type ListAppSetting or convert a list of dicts of settings via ListAppSetting(list_of_dicts).
Is more docs needed?
Question, if we'd want to use OVERRIDING_APPS / EXTRA_APPS. we'd want to do this without environment variables but add the relevant structs directly in our settings.py. Right now the only way I can see that we could do that is by calling
update_settingsourselves, right?
update_settingsneeds to be run in the final, outermost settings-file yes. Rememberglobals(). No env-vars needed.
update_settingsnow takes the boolean flag override (default: False).It would be nice if we could do something like this: geant_settings.py
from argus_htmx import app_settings update_settings(app_settings)That way we can have the magic, while still being in control of it :)
Just copy the stuff from argus.htmx.settings into the geant settings, then override what you need. If you have your own geant app_settings you could run update_settings twice.
Either make an instance of type ListAppSetting or convert a list of dicts of settings via ListAppSetting(list_of_dicts).
Is more docs needed?
I'll play around with it, see if I actually want it 😅 . On one hand I want argus_htmx to tell me what settings it wants (additional apps, middleware, context processors, etc). This also changes regularly (additional context processors for example) and I don't want to copy everything. But I also want some more granular control (swap out our own context processor for theming or whatever).
I'm having some trouble with argus-htmx depending on stuff from the spa-settings that it doesn't actually need...
Do docs/README in https://github.com/Uninett/argus-htmx-frontend need a cleanup round after this PR is merged?
Yes! We need a release of argus-server, "vN", with this, then a release of argus-frontend-htmx that depends on "vN" and that has its readme updated with new instructions. I'll make an issue.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code