Skip to content

Conversation

@abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented Apr 24, 2023

Why make this change?

What is this change?

  • Added a new Class to enable merging of two different configs.
  • This merge feature is only applicable with dab start.
  • If the user provide a config file it will still be honored to ensure backward compatibility.
  • When the user doesn't provide a config file with dab start. It will check if DAB_ENVIRONMENT is set. if yes, then it will try to merge the dab-config.json with the dab-config.{DAB_ENVIRONMENT}.json and use the generated merged file dab-config.{DAB_ENVIRONMENT}.merged.json to startup the engine.
  • If only dab-config.{DAB_ENVIRONMENT}.json is available, it will be used. if not it will check for dab-config.json. if that is also not available, an error will be thrown.

How the merge is performed?

  • if the type is same, then object from second config is given preference. merging of json object is done recurrsively.
  • if the second object is null, property from first config is given preference.
  • in case of JsonArray, the elements from second config completely overrides that of the original config, i.e. the merged config will have elements in the array from only second config.

How was this tested?

  • Unit Tests

Sample Request(s)

  • set the DAB_ENVIRONMENT variable in your environment settings. for ex: Development.
  • you can keep your local changes in a file called dab-config.Development.json. It will have the connection string and other information that is for local development.
  • And you can have a file dab-config.json which can act as base file.
  • when we do dab start the merge will be performed, and the generated merged file dab-config.Development.merged.json will be used to fire up the engine.

Example Scenarios:

Case1:

Environment value: $env:DAB_ENVIROMENT=""

command: dab start

Files in Directory:

  • dab-config.json
  • dab-config.DEVELOPMENT.json

Config Used for startup: dab-config.json

Reasoning: Environment value not setup

Case2:

Environment value: $env:DAB_ENVIROMENT="PRODUCTION"

command: dab start

Files in Directory:

  • dab-config.json
  • dab-config.DEVELOPMENT.json

Config Used for startup: dab-config.json

Reasoning: dab-config.PRODUCTION.json is not present in the directory

Case3:

Environment value: $env:DAB_ENVIROMENT="DEVELOPMENT"

command: dab start

Files in Directory:

  • dab-config.json
  • dab-config.DEVELOPMENT.json

Config Used for startup: dab-config.json + dab-config.DEVELOPMENT.json (Merged)

Reasoning: both dab-config.json and dab-config.DEVELOPMENT.json is present in the directory

Case4:

Environment value: $env:DAB_ENVIROMENT="DEVELOPMENT"

command: dab start -c dab-config.json

Files in Directory:

  • dab-config.json
  • dab-config.DEVELOPMENT.json

Config Used for startup: dab-config.json

Reasoning: If config is provided by the user it will always get precedence irrespective of the ENVIRONMENT value.

@aaronpowell
Copy link
Contributor

Could I make a request that we be really cautious on this work as I'm getting close to completing #1402 which will see a major overhaul of the config type system in place, which may impact the way that merging needs to be undertaken.

@abhishekkumams abhishekkumams marked this pull request as ready for review April 25, 2023 04:06
@ayush3797
Copy link
Contributor

ayush3797 commented Apr 27, 2023

Please update the comment here: it is no longer only used for testing. Probably explicitly write what it is initialized with.

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking for few answers

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last few nits and question based on latest changes

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this capability!

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

LGTM, thanks for clarifying my questions!

@abhishekkumams abhishekkumams merged commit 2cfb505 into main May 5, 2023
@abhishekkumams abhishekkumams deleted the dev/abhishkkuma/merge_config_files branch May 5, 2023 06:34
@Aniruddh25 Aniruddh25 linked an issue May 8, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Issues with multiple config files Should we do union of configuration properties specified in dab-config.json and dab-config.<Environment>.json?

7 participants