Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(config)!: re-design and clean-up configuration #979

Merged
merged 42 commits into from
Dec 23, 2022

Conversation

Alex6323
Copy link
Collaborator

@Alex6323 Alex6323 commented Dec 13, 2022

Description and Motivation

This PR changes the way how Chronicle can be configured. This was motivated by the fact that we defined defaults in multiple locations and naturally ... conflicts appeared and bit us ... some place. Although those defaults were in theory (if they had been consistent) documented in the config.template.toml file, we think those are better documented in the CLI itself (if you invoke --help). Another problem with such a file is that users were supposed to make a copy of that file and change it to their liking. This resulted in the config.toml being invalidated earlier than it needed to be since often times no parameter that the user actually adapted was conflicting with the newer version. This PR changes the preferred mode of configuration for Chronicle as it makes everything configurable through the CLI itself and also limits supported environment variables to credentials basically (which are more unlikely to be deprecated). Together with the focus of Chronicle as a Docker deployment, this also allows for making our docker-compose.yml the main location of configuration as it's no longer needed to copy over the template file.

Linked Issues

Notes to Reviewer

As a reviewer, please pay particular attention to the following areas when reviewing this PR and tick the above boxes after you have completed the steps.

Config Changes

  • Ensure proper order in which CLI and config arguments are applied.
  • Ensure that config changes work with individual build features by running cargo ci-check-features.

@Alex6323 Alex6323 marked this pull request as ready for review December 14, 2022 09:52
@Alex6323 Alex6323 changed the title Refactor/config/config refactor refactor(config)!: redesign how Chronicle can be configured Dec 14, 2022
@Alex6323 Alex6323 changed the title refactor(config)!: redesign how Chronicle can be configured refactor(config)!: re-design and clean-up configuration Dec 16, 2022
src/bin/inx-chronicle/cli.rs Outdated Show resolved Hide resolved
@Alex6323 Alex6323 force-pushed the refactor/config/config-refactor branch from c0fb35b to e0a7e71 Compare December 20, 2022 10:02
.github/workflows/_test_int.yml Outdated Show resolved Hide resolved
src/bin/inx-chronicle/cli.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@grtlr grtlr left a comment

Choose a reason for hiding this comment

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

Please make sure cargo ci-check-features command passes.

src/bin/inx-chronicle/cli.rs Outdated Show resolved Hide resolved
src/bin/inx-chronicle/main.rs Outdated Show resolved Hide resolved
src/db/mongodb/collection.rs Outdated Show resolved Hide resolved
@grtlr
Copy link
Contributor

grtlr commented Dec 23, 2022

Could you please also do #1000 as part of this PR? 🙏

README.md Outdated Show resolved Hide resolved
@grtlr grtlr merged commit af57aa3 into main Dec 23, 2022
@grtlr grtlr deleted the refactor/config/config-refactor branch December 23, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants