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

Remove unnecessary config params required to start fully functional Horizon instance. #3719

Closed
bartekn opened this issue Jun 24, 2021 · 2 comments · Fixed by #3783
Closed
Labels

Comments

@bartekn
Copy link
Contributor

bartekn commented Jun 24, 2021

To improve easy of use of Horizon @Shaptic suggested to decrease number of mandatory config params required to start fully functional Horizon. Below is a list of config params required to start ingesting instance with some ideas how we can change make them optional:

  • DATABASE_URL - There's probably nothing we can do because Horizon requires a Postgres DB. We could potentially use a default postgres://localhost:5432/horizon?sslmode=disable but errors about invalid credentials can be more confusing than asking for setting this param.
  • INGEST - This is currently false by default but we could make it true so starting Horizon without the param will always start ingestion.
  • STELLAR_CORE_URL - The default ingestion is using Captive-Core so we could default to Captive-Core HTTP port.
  • STELLAR_CORE_BINARY_PATH - We can require this param only if stellar-core binary is not available in $PATH.
  • CAPTIVE_CORE_CONFIG_PATH - We discussed it internally and it's tempting to provide a default config file with tier 1 validators. There's a risk that people will never provide a production ready config file.
  • HISTORY_ARCHIVE_URLS - The values of history archives could be extracted from CAPTIVE_CORE_CONFIG_PATH.
  • NETWORK_PASSPHRASE - The value could be extracted from CAPTIVE_CORE_CONFIG_PATH.

To sum it up, it should be possible to start fully functional Horizon by only setting a single config param: DATABASE_URL. While it will definitely improve easy of use there are some risks connected to using default values in production. We should make sure we understand the risks before implementing this.

@Shaptic
Copy link
Contributor

Shaptic commented Jul 7, 2021

Love this follow-up! Especially --ingest=true by default. Definitely will make it easier to onboard new users/devs/ops to running Horizon.

I'd expect HISTORY_ARCHIVE_URLS to be the most contentious one of these to overcome, given previous discussion about the format differences between Captive Core and Horizon. On the other hand, since we're moving into a world now where Captive Core's config is more explicitly disjoint from pure Core (e.g. BUCKET_DIR_PATH is unsupported), we might be able to overcome that now.

@Shaptic
Copy link
Contributor

Shaptic commented Jul 21, 2021

As a follow-up after our discussion in the team meeting, we could avoid the "nobody reads their configs" problem by not providing defaults there (or providing a package for it e.g. apt install stellar-horizon-default-captive-core, but I'm not sure I like that). Extraction should still be just as possible for the other parameters.

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

Successfully merging a pull request may close this issue.

2 participants