We used Spring Cloud Config extensively in the past on a project. Having the config audit trail is incredibly useful indeed, but we shouldn't underestimate the additional infrastructure complexity required for this solution. There were 2 primary concerns: - local development - keeping environment configurations in a separate repository
Speaking up for the developers, there always should be an easy way to run Fineract locally from our IDEs, without any additional tricking, without packaging to docker or running a git server, running a spring cloud config server, etc. Developing Fineract as a daily base is slow enough as it is (working on it though..), so I think we should avoid any additional complexity or slowdowns to local development. The other concern was that we kept the configurations in a separate git repo (to avoid creating subtrees or other git-fu), and maintaining configurations there was noticeably more cumbersome. I think a hybrid approach could be ideal, where the application is configured locally with the right spring profile, and uses config server for the real environments. Kristof On Mon, Sep 16, 2024 at 12:11 AM Aleksandar Vidakovic < chee...@monkeysintown.com> wrote: > ... @Ádám Sághy <adamsa...@gmail.com> concerning sync/consistency across > instances: not a problem if we use the best practices tools like ConfigMaps > (if we are in Kubernetes) or Spring Cloud Config (could also be used in a > Kubernetes environment, but doesn't require one like ConfigMaps). One thing > that is really neat with Spring Cloud Config is the support to use a Git > repository to hold all your configurations (aka application.properties). > Changes are propagated immediately and you have a "natural" audit trail of > any changes that are made where as config changes in a database table are > "destructive" (read: when you overwrite a config entry in a database table > there will be no trace of the previous value)... so, the Git backed configs > give you even an easy "rollback" in case something goes wrong. > > For more convincing see: > https://docs.spring.io/spring-cloud-config/docs/current/reference/html/ > > ... but even if you run your multiple Fineract instances with "plain" > Docker containers and maybe with Docker Compose (for convenience) then you > can just create 1 application.properties file outside of the Docker images > (aka copy the original one and save it to the file system); then apply > changes as necessary for your deployment and finally mount the same > application.properties file from the local file system into the running > Docker containers... and if each container runs on a different physical > machine then just create a shared volume that contains the > application.properties... voila, all instances have the same configuration. > > Aleks > > On Sat, Sep 14, 2024 at 3:18 AM James Dailey <jamespdai...@gmail.com> > wrote: > >> Cool Adam. Thanks Alexander for your thoughts too. >> >> I personally don’t have any deep insights into this, but I would ask: >> would the developer (future Adam) a year from now be happy with this >> choice? >> >> I do remember the past debates about abstraction of configuration and >> initially it was all in text files, which had to be carefully edited in xml >> but was human readable. >> >> So incrementally better is fine with me, as long as it makes things more >> maintainable than before. >> >> I also don’t get the concern about different layers of configuration as >> long as the layering has clear inheritance and ordering. >> >> Thanks, >> James >> >> >> >> >> >> On Fri, Sep 13, 2024 at 10:43 AM Ádám Sághy <adamsa...@gmail.com> wrote: >> >>> Hi Aleks, >>> >>> Thank you for sharing your thoughts! >>> >>> I believe your ideas have merit, and many could positively impact the >>> project. However, I don't think the implementation is as straightforward. >>> If these configurations are managed externally, they need to be synced and >>> updated across all running instances; otherwise, we risk inconsistencies >>> between them. Additionally, the time required for synchronization across >>> instances should be considered. >>> >>> Please don't misunderstand—I'm not a big fan of the current situation >>> where we have: >>> >>> - Global configurations in the database >>> - Configuration in application.properties >>> - Configuration in the tenant store database >>> >>> I definitely agree that the latter two should be merged, and we should >>> move away from the third option. >>> >>> However, I’m still uncertain about the first one due to the challenges >>> of synchronization and consistency across running instances. That said, I’m >>> open to being convinced. >>> >>> In the long term, I agree that we should invest time and effort into >>> implementing the changes you've suggested. For now, though, I was aiming to >>> take a smaller step toward resolving a current issue that's causing >>> problems and providing a better way to manage global configurations. >>> >>> Best regards, >>> Adam >>> >>> On 13 Sep 2024, at 18:57, Aleksandar Vidakovic < >>> chee...@monkeysintown.com> wrote: >>> >>> 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 >>>> >>> >>>
