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

RFC: Creating Horizon default config file #4966

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

urvisavla
Copy link
Contributor

@urvisavla urvisavla commented Jul 17, 2023

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

This pull request proposal in the RFC stage

Create a Horizon config file that serves as a single location for managing all configuration parameters easily.
The config file should address the following:

  • Documenting individual parameter
  • Specify default (recommended) values for parameters
  • Grouping of related parameters
  • Include documentation and official links
  • Rename parameters to be more intuitive (Optional)
  • Identify and remove unnecessary or deprecated parameters for cleaner configuration
  • Specify mandatory options for specific Horizon instance types, such as setting ENABLE_INGEST to true for ingestion instance

Why

  • It provides a centralized and convenient way to manage all configuration parameters in one place, simplifying the configuration process
  • Grouping related parameters improves organization and helps users find and better understand the parameters
  • Including official documentation links for each parameter to help users in getting up-to-date information
  • Separating default values from the code allows for easier modification of default configuration settings without code changes

Alternative Recommendation

Instead of a config file, presenting the configuration parameters in a tabular format, grouping parameters by relevance, on the public documentation site to provide a complete reference.

@sreuland
Copy link
Contributor

I think is great round-up with ability to show richer context than -h may show, just wondering, is it potentially much overlapping content to maintain the same copy here and in flags definitions? would we display this file as the output for the -h help on command line?

wondering if something like this is then recommended for users to take as template, edit per their deployment and either import the file into their environment variables? or maybe we add a new --config-file flag to horizon to load this.

@urvisavla
Copy link
Contributor Author

I think is great round-up with ability to show richer context than -h may show, just wondering, is it potentially much overlapping content to maintain the same copy here and in flags definitions? would we display this file as the output for the -h help on command line?

My understanding is that we want to deprecate command line arguments(#4915) for config parameters and support environment variables only approach. So In that case, the config parameters will not be displayed in the --help output for Horizon.

wondering if something like this is then recommended for users to take as template, edit per their deployment and either import the file into their environment variables? or maybe we add a new --config-file flag to horizon to load this.

Here are some options for integrating the configuration file into Horizon setup:

  1. Standard format with --config-file flag:

    • Use a standard format like TOML to create the config file.
    • Introduce a new command-line flag --config-file flag for Horizon, which can be used to specify the path to the configuration file
  2. Environment variables with wrapper tool:

    • Create the configuration file as a shell script that sets environment variables.
    • Develop a wrapper tool/script that users can use to start Horizon making sure that the environment variables are properly set.

In any case, we can include the Horizon default configuration file with the distribution, allowing users to customize it as per their needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants