Hi Adam, ... my question would be: why do we need this service (read: rhetoric question)? I know that the initial idea here was to introduce the capability to change configuration without any downtime... but is it really that useful? I think: - it's confusing to have 2 different configuration concepts (actually 3 if we consider the tenant configuration database)... there is application.properties, the global configuration service... and the tenant configuration - nowadays there are better concepts to get the same functionality if not better functionality (Kubernetes Config Maps, Spring configuration server...) - changing configuration is/can be a delicate process... that's why the Spring people reload the whole application context if the configuration changed... for a reason I think
Just imagine you are a devops person and want to set up Fineract for production in a reproducible way... maybe with Terraform... Ansible... bash scripts... doesn't really matter. Alright, so you take care of application.properties (or the environment variables... boils down to pretty much the same), but now suddenly you have to switch context from a file (application.properties) to your database (and wait until it's available etc. etc.) and additionally poke values into the database (plain sql statements? with liquibase?...) to make your production environment work (I know that people use the web UI to do this manually... well, I'd argue that that's not devops). BTW: same argument applies to the tenant configuration DB... should also be defined in application.properties In short: instead of dragging that global config service around, why not make an effort and move configuration related stuff into a single source of truth, application.properties and get rid of the service entirely? And while we are at it we should do the same with tenant configuration (or at least offer it as an optional way of running your Fineract). Cheers On Fri, Sep 13, 2024 at 6:16 PM Ádám Sághy <adamsa...@gmail.com> wrote: > Dear Fineract Community, > > I would like to bring your attention to my recent PR: > https://github.com/apache/fineract/pull/4057. I would greatly appreciate > your feedback and thoughts on the matter. > > Historically, global configurations were introduced into the system via > Flyway (up to version 1.6) and later through Liquibase scripts. In many > cases, the ID field value was explicitly defined, effectively hardcoding it. > > Currently, the global configuration API provides the following > capabilities: > > - Fetching entries by ID > - Fetching entries by name (this functionality was previously > introduced) > - Updating entries by ID only > > *This PR adds the ability to update global configuration entries by name* > . > > > - Since the name field is unique for each global configuration, it > serves as an ideal candidate for identifying entries. > > *Additionally, this PR modifies the integration tests, changing the > retrieval and updating of global configurations to rely solely on the entry > name*. > > > - This change improves readability and helps prevent issues caused by > ID changes or inconsistencies due to custom configurations. > > *Why should we prefer using the name instead of the ID?* > > - Hardcoded IDs in Liquibase scripts for global configurations can > lead to conflicts and inconsistencies, especially when past entries have > been deleted or when projects have introduced custom configurations. These > issues may arise when new global configurations are added. > - I propose that, moving forward, we avoid providing hardcoded IDs for > new global configurations. Instead, we should rely on the database’s > sequence generator or identity solutions to assign IDs. This practice could > also extend beyond global configurations, as hardcoded IDs are generally > not recommended. > > I look forward to hearing your thoughts on this proposal. > > Best regards, > Adam >
