... just to clarify: I consider any of the configuration alternatives I suggested a breaking change... i.e. it's not an "all batteries included" approach that up until now Fineract propagated...
Agree with your (@Arnold, @Kristof) suggested step by step replacement... would be also unfair to current users to rip everything apart at once. Cheers On Mon, Sep 16, 2024 at 12:17 PM Arnold Galovics <galovicsarn...@gmail.com> wrote: > Hey guys, > > I tend to agree with Kristof here. > > I also have a similar xp like him and Spring Cloud Config on brochure > looks really good but when using it, there's the extra complexity. > > I'd go even further what Kristof just outlined, bringing another pain > point we had with it. Bean reloading. As you said Aleks, you can totally > modify the configuration runtime and it'll be picked up by the > applications. But, it's not always that easy. Whenever we "hack" something > in Spring or just not use it according to their suggestions, it can break > during reloading, or doesn't even reload beans with the new configurations. > This can lead to sleepless nights of investigating something that really > isn't there but occurs when changing the configurations runtime. > > I think we all can agree that Fineract has some "nasty" Spring stuff that > we're not proud of and will eventually break some of Spring Cloud > Config's offerings. > > I'd suggest going step-by-step. > 1. Let Adam's PR in, so we have better support for Global Configs. > 2. Start decommissioning Global Configs and move them into application > properties. > 3. As soon as everything is in application properties. Remove Global > Configurations entirely and propose a migration path for existing users. > 4. Only here, let's think about Spring Cloud Config - don't introduce it, > but start the discussion. > > Hope that makes sense. > Best, > Arnold > > > On Mon, Sep 16, 2024 at 11:43 AM Kristof Jozsa <kristof.jo...@gmail.com> > wrote: > >> 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 >>>>>> >>>>> >>>>>
