Issues/2023 11 16

[svenvandescheur]

No description provided.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0af2a12) 92.85% compared to head (059cad5) 92.85%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #849   +/-   ##
========================================
  Coverage    92.85%   92.85%           
========================================
  Files          759      759           
  Lines        26312    26314    +2     
========================================
+ Hits         24431    24433    +2     
  Misses        1881     1881           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alextreme]

Not sure why this would be necessary, please do a review with a colleague first

[pi-sigma]

@svenvandescheur Could you add a description that explains what the issue is? If this is about Mac-specific library paths, you should put the code in your conf/local.py, which is intended for machine-specific settings. Depending on the issue, we could update the installation instructions with a subsection for Mac users (in this case, feel free to include any other Mac-related problems you encountered), but the general settings shouldn't contain machine-specific stuff.

[svenvandescheur]

This is equal to: open-zaak/open-zaak#1472

This adds the possibility to configure the GEOS_LIBRARY_PATH and the GDAL_LIBRARY_PATH Django settings using a .env (dotenv) file. It does not alter the default setting values.

On Mac system, the relevant library may not be installed on the location expected by default. Django provides an escape hatch to alter the path using a setting and point it towards the correct files.

Setting this using a local.py might work (and support wont change), however for modern setups, using a .env is more favorable and since most relevant settings (like those for database connectivity) already support environment variables across various projects, I think it's more sensible to support using configuration this way.

[pi-sigma]

@svenvandescheur The database settings (among other things) are configurable via env variables because they differ in production (different clients use different settings). There is not point in making these paths configurable for Mac users because all production servers run Debian. That's what local.py is for, to adapt the settings to your personal environment. I get that it's annoying to stumble over this with no indication on how to solve it; hence the suggestion to update the installation instructions with a section for Mac users.

[svenvandescheur]

@svenvandescheur The database settings (among other things) are configurable via env variables because they differ in production (different clients use different settings). There is not point in making these paths configurable for Mac users because all production servers run Debian. That's what local.py is for, to adapt the settings to your personal environment. I get that it's annoying to stumble over this with no indication on how to solve it; hence the suggestion to update the installation instructions with a section for Mac users.

I don't see why we should use local.py over .env. The latter is more flexible (and modern/standars based) over the older local.py which is a leftover from historic setups. They are not mutually exclusive and this patch will not prevent anyone from configuring this, or anything else using a local.py file. Additionally, this change will not effect any other (existing) setups (the defaults still apply here).

At tis point I don't see any reason not to support this and I do see reasons why we do want to support this (setting the envar globally for instance) is only to keep in sync with other open-X projects.

[pi-sigma]

@alextreme Discussed with @svenvandescheur. The library paths are indeed relevant for Mac/Windows users, but the bigger issue is how we structure our settings, in particular, how we use the settings in base.py vis-a-vis development settings. Do we want as few settings as possible in base.py and extend/override in local.py, which means every developer needs to do that for themselves; or do we want to include these options in base.py and make them configurable via env variables? I don't mind either way, but it would be good to make a principled decision about how we want to handle this sort of thing going forward.

[alextreme]

@pi-sigma in principle I'd adhere to using local.py for development settings and only use envvars for relevant deployment-time settings (either dockerized or not). The reasoning behind this is mostly due to envvars in Kubernetes setups being shared between services on the same node which can cause surprises ( kubernetes/kubernetes#89033 ), you only want to listen to envvars if you need to.