search

maykinmedia / django-setup-configuration

MIT License
0 stars 0 forks source link

Default setup_configuration value should be False #10

Open alextreme opened 2 months ago

alextreme commented 2 months ago

For the individual configuration items, e.g.

SITES_CONFIG_ENABLE NOTIF_OPENZAAK_CONFIG_ENABLE OPENZAAK_NOTIF_CONFIG_ENABLE ADMIN_OIDC_CONFIG_ENABLE

etc etc. For all of them.

When I run setup_configuration now if I just want to configure ADMIN_OIDC_CONFIG_ENABLE I will need to explicitly set all the others to False. This is very annoying.

Tasks

sjoerdie commented 2 months ago

In overleg met Joeri op naam van Sergei.

sergei-maertens commented 2 months ago

I'm going to need a lot more context because this doesn't ring any bells for me

sjoerdie commented 2 months ago

Of course.

So for example, openzaak: https://open-zaak.readthedocs.io/en/latest/installation/config/openzaak_config_cli.html?highlight=setup_configuration

For demonstration sake I have unset SITES_CONFIG_ENABLE, OPENZAAK_DOMAIN, OPENZAAK_ORGANIZATION.

Now when the init container with setup_configuration runs it will result in an error:

CommandError: Prerequisites for configuration are not fulfilled: Site Configuration: OPENZAAK_DOMAIN, OPENZAAK_ORGANIZATION settings should be provided
openzaak-8496bc744d-zld7t                                0/1     Init:CrashLoopBackOff   4 (20s ago)      4m11s

For openzaak there are not that many yet and the chart sets it to false by default: https://github.com/maykinmedia/charts/blob/b0039064130c3c24269c83e7a150f35be7a43b1a/charts/openzaak/values.yaml#L17

When a new setup_configuration item is added with a new release it would break the existing setup_configuration config because this new item would default to True when not set. You would need to explicitly set this new item to False to prevent the command from failing.

sergei-maertens commented 2 months ago

@joeribekker I think this is better suited for Anna / team bron that actually define the setup configuration items and maintain that library?

alextreme commented 2 months ago

Discussed with Sjoerd, and the following enable_setting 'True' default value should be set to False: https://github.com/maykinmedia/django-setup-configuration/blob/0c5557799f3bb9f340112dfb4ca7b99d35986fe4/django_setup_configuration/configuration.py#L42C55-L42C59

annashamray commented 1 week ago

I can do it in django_setup_configuration, but it won't affect Open Zaak configuration much, since we define all *_ENABLE settings there (https://github.com/open-zaak/open-zaak/blob/88b6e9ff734d604255837c0e8d18ff5e8a9ce6d2/src/openzaak/conf/includes/base.py#L275 )

So it should be done in all components which use django_setup_configuration

annashamray commented 1 week ago

I've added tasks to this issue to keep track of the progress