0008-single-config-for-all-modules.md

[jiachen1120]

@NicholasAzar @stevehu This RFC is related to issue API-48. And I created
a new branch in networknt/light-4j called feature/single-config-for-all-modules,
Look forward to your reviews.

Existing configuration modules can only load config separately through
separate configuration files for each module. However, some developers
want to be able to configure all modules with a single configuration
file. For example, now if we want to configure the service module and
the mask module, the server.yml and mask.yml need to be provided. But
the feature discussed in this document is to put all configuration
information into a file called application extend with yaml, json or yml
along with the following format:

server:
    # some contents for server
mask:
    # some contents for mask

The codes can be checked over there:
https://github.com/networknt/light-4j/tree/feature/single-config-for-all-modules

[stevehu]

I think the first thing first is to discuss the reason we are doing this. What is the pros and cons of each approach? The reason we are adopting configuration per module is due to the light-4j is a plugin architecture and not necessarily every module is developed by our team. Also, for each individual application, the number of modules included is dynamic. If we put all configurations file into a big one, we need to introduce another layer in the hierarchy. Also, the file might be too big to manage with API specification and status merged together. How about those binary files like keystores and truststores?

[jiachen1120]

Yes, bringing all the files together can make the files too large and difficult to manage. Perhaps these developers who need this feature want to be able to bring together a subset of commonly used configuration files that do not need to be modified frequently. And some of the modules that are optionally use, or files that are not well integrated, continue to use the previous configuration way? In the implementation, the way I provide is to continue to load from the module's unique configuration file if the corresponding configuration cannot be found in this large configuration file. Is this useful? I think maybe its just to cater to some developers' preferences? Look forward to your opinions.

[stevehu]

One of the benefits to having individual config file is to allow the config to be loaded at different levels. For example, you can package the config file into the module and overwrite it in the application and externalized config folder. If we merge them together, then you lose the flexibility. Could you please ask the reasons from the requestor? We need to understand the use cases they have and make the final decision. If this is just a personal preference, I don't think we should do that. Thanks.

[DSchrupert]

@stevehu I'm not sure I understand why not... if there are some developers who have a personal preference to use one file, should they not at least have the option? What is the framework benefitting by making them do something they don't like to/want to do?
I want to also emphasize that this is not changing any option to use individual config files as you can do now, it will just add an additional option for those who want to use it. And as @jiachen1120 mentioned, they don't have to put ALL config in the one file.. if they want to put a subset that's their choice.
Also, this would not include the api specification, logback, or cert/truststore files.

[stevehu]

As our target is microservices, we want to make sure that the final delivery is as small as possible and as simple as possible. When we are adding new features, the first question we need to ask is what is the use cases. What kind of issues the user is facing and if we have alternative options. In this way, we can ensure that our platform is driven by use cases and resolving real problems. Personal preference is very subjective and the more users we have, we will have more preferences and we cannot make everybody happy as there might be conflicts between developers. Also, our effort might be wasted by addressing these personal preferences which might be used by only one person who is asking for it. I would rather put our effort on real use cases to provide real value for our customers. This is why I am asking the use case from the original requestor. There might be a very good reason for the request but we need to understand it.

If there are use cases behind it, I would suggest making the Config as an interface instead of an abstract class as of today. And create another module for single-file config. By default, the Config will use the current implementation. If the user wants to use a single file, then he/she needs to include another module and update service.yml to specify the implementation. In this way, for the vast majority of users, they don't need to include some classes they don't use and suffer the extra check for the applicaton.yml during server startup.

[DSchrupert]

I appreciate the time you put into your response, hopefully my answers to each of your concerns helps clarify my position on this request.

As our target is microservices, we want to make sure that the final delivery is as small as possible and as simple as possible.

This change is including a single 60 line class file and a few minor changes. Size of the delivery artifact is not impacted. In regards to simplicity, I suppose most additional features and options would add additional complexity, but in this case it's really not much. We're just adding an option for developers to aggregate their many files into a single one, not introducing machine learning.

When we are adding new features, the first question we need to ask is what is the use cases. What kind of issues the user is facing and if we have alternative options. In this way, we can ensure that our platform is driven by use cases and resolving real problems.

Does every change to the framework need to be accompanied by a use case? Why is developer experience not enough? If some developers are complaining that they're finding it difficult to manage a directory filled with small single purpose files and would prefer to have a single file with clear delineated sections, what benefit is it for us to say that they should not have the option? Are we benefiting by restricting developers to do something they would prefer not doing?

Personal preference is very subjective and the more users we have, we will have more preferences and we cannot make everybody happy as there might be conflicts between developers.

Of course personal preference is subjective.. it's pretty much the definition. But to extrapolate this request to say that we now have to cater to the preferences of every developer in the future is unreasonable. This is not the request of a single developer, this is continued feedback ive received from multiple developers from multiple teams. And to say that it could lead to conflict? How is this possible? We're not changing anything in regards to how config is currently managed. If you want to keep your config in multiple files as you are now, you're more than welcome. If you're part of the few so far who prefer a single file, you also have the option. Where do you expect the conflict to come from?

If there are use cases behind it, I would suggest making the Config as an interface instead of an abstract class as of today. And create another module for single-file config. By default, the Config will use the current implementation. If the user wants to use a single file, then he/she needs to include another module and update service.yml to specify the implementation. In this way, for the vast majority of users, they don't need to include some classes they don't use and suffer the extra check for the applicaton.yml during server startup.

So instead of having users "include some classes they don't use" (1) and "suffer the extra check", you want to potentially rewrite the way config is currently managed and add another module to the platform? For what? How many other options to manage config are there other then "1 file" or "multiple files". The config server already has its hooks in as far as i understand..

If there's really such a strong opinion about not including this change, perhaps we could settle for including it as a beta trial-run type feature? And potentially rolling it back if we receive backlash from the user base? This way there would be no commitment of supporting it in the future if it becomes too difficult to maintain and I would be better able to respond when future developers request the same feature.

[stevehu]

@NicholasAzar Thanks a lot for providing the detailed response. The discussions help all the people involved to understand how the new feature is introduced. The RFC and the discussions also provide the raw material for the documentation once the feature is released. The discussion also provides the historical info on how the config module is evolved.

Usability and personal preference are totally different things. If there are too many people complain about the current implementation, we need to address it. Would you be able to elaborate the exact complains they have? We want to know what is the exact issue in order to address the issue correctly. This is the use case I was referring to in the previous post. Personal preferences are purely subjective and 100 people might want it to be implemented in 100 different ways and we cannot make all of them happy. And some of the ideas might be conflicted between each other. As you see I am open to address it if I understand the rationale behind it. We just need to come up with a solution that is not very intrusive.

Let's dig it a little deeper in term of usage with all the latest features we have implemented. First, we all know it is not possible to merge every config file together. So even we implement it there are still several files at least. Second, I am assuming the users who want this feature is to reduce the complexity to manage multiple changing config files in the externalized folder. One file would be much easier to manage within the delivery pipeline than multiple. If this is the case, the new values.yml has already addressed the issue. They can package all config files inside the application as application-level configuration and only externalize one values.yml which is exact they want. Last, my assumption is that nobody cares how many config files if they are not externalized. Please correct me if there are use case against my assmption.

Now, let's evaluate the design options. It might seem easier to add another layer on top of the current implementation; however, the current implementation is already so complicated with the recently added features. I got some complaints from users about the stability and backward compatibility of the config module. @ddobrin has just opened another issue today to address some gaps in the new feature we just released. I feel uncomfortable to just put one more layer into the current implementation as a single file and multiple files are totally opposite directions.

When the config module was designed, the idea is always to build an interface with multiple implementations so that we can have filesystem based config, database-based config, config from an external server or different vault services. The reason it was implemented as an abstract class is due to the lack of IoC support at that time. Now we have the service module that you can specify the implementations for one interface. If I were to implement the config module today, it must be an interface with multiple implementations.

The entire platform design is following some principles. Sorry. I don't have details written down yet but only the general principles. We want each component to be as simple as possible so that developers can easy to understand. For users, they would be easy to reason about. They only need to address a single capability and do it well. More features can be implemented in a separate module that can be injected. This is the key difference between microservices and monolithic. Like Lego blocks, the more small shapes you have the more flexibility you have.

My understanding is that one user can only choose multiple config files or a single config. In this case, I really don't want to merge the two in-compatible feature into the same module and make it configurable. There is already config of config already and we really don't want to increase the complexity of this module. For most users, it is already too hard to learn.

Although it takes a little bit more time to switch the Config to an interface, it is very crucial to address this technical debt. It will open the door to implementing other solutions and allowing customers to provide their customized solutions. What do you think?

[ddobrin]

@stevehu, @NicholasAzar : please allow me to propose something.
I will share offline a values.yml file with you and then provide a subsequent comment here in the next few minutes

[ddobrin]

@stevehu @NicholasAzar :
we need to look into the ways different development teams, in teams building multiple APIs, where sharing utility modules and cross-cutting concern modules are the norm.

Providing individual config files per module provides the right amount of independent management and maintainability.

Let's think how teams use these modules: my client has developed a good number of shared modules for various tasks (won't get into details) Each has a set configuration and the developers can override the default values in the values.yml.
In most(many) APIs, these won't get overridden and the team would not be preoccupied with them.

Let's think in steps:
[1] The values.yml file already provides, as @stevehu mentioned, a single configuration file, where one can override every single config element, if one chooses to do so.

[2] The configuration based environment variables allow you to define variables and also provoide a default. Ex.:
enableHttp2: ${server.enableHttp2:true}

values.yml sets the variable value, however a default was already provided:
server.enableHttp2: true

[3] The light-codegen will support a feature to generate a "most commonly-used" set of variables, driven by a flag, if somebody chooses to generate one.
An issue will be created in the light-codegen module to asssist with this topic.

[4] My client has asked for this file to be generated, saves teams some initial work.

Proposed solution:

1. light-codegen generates the config files as specified in the Rocker RAW files with a variable name and the current default. This makes it perfect backwards compatible.

It generates the variable in the format: ., which is unique across all configs
Ex.: server.yml defines
server.enableHttp2: true

2. dev teams, or ops teams, can override any value, without worrying about the variables themselves, in the values.yml file or by setting them at command-line.

Advantage:

1. No additional code is required
2. Backwards compatibility is maintained
3. Config files are built into the fat API jar, and the -Dlight-config-dir will contain one and only one file if you choose to use it: the values.yml file.
4. The functionality is being achieved without any code change.

Would you agree on this approach and we have a sensible resolution, without having to complicate the Config module anymore?

[ddobrin]

Review provided in comment made on 25.01.2019 @9:20 AM:
proposed a config based solution which does not require any additional code change for a single cofnig

[DSchrupert]

Thanks again for your time and patience with me on this @stevehu

I feel I already outlined my perspective on the issue as clear as I can. As you are the decision maker within this repository i will have to accept the direction you wish to take it, despite my disagreement.

[stevehu]

@NicholasAzar We are still in discussion and there is no final decision made yet. @ddobrin and I just commented based on the assumption that the users who want the feature is trying to simplify the externalized config files to make the DevOps pipeline more smooth. If you have more use cases, let's bring it up to discuss. As you are closer to the end-users, I really need your teams' help to provide background information and use cases in order to understand. The RFC document and the in-depth discussion in this thread help me a lot and really appreciate everyone's effort here.

[ddobrin]

Agreed.
I will address the proposal in the above thread as part of the issue:
networknt/light-codegen#208.

If it addresses your use case @NicholasAzar and @jiachen1120, then it is good.
If there is more required, it can get addressed as @stevehu is saying.

Let's take it in steps. I'll comment again once it is implemented

[DSchrupert]

it's not clear to me what @ddobrin is suggesting and will wait for the implementation to come out before commenting on it

[DSchrupert]

Closing as discussion completed per note in networknt/light-4j#367