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 the config supporting running horizon without a captive stellar-core #498

Merged

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Sep 19, 2023

What

Remove the config supporting running horizon without a captive stellar-core. The only way to run horizon will be with a captive stellar-core.

Why

We have two ways to run horizon, with and without captive core. In the quickstart image the default way is without captive core. Outside of the quickstart image the recommended way is with captive core. As far as I can tell we only support the without captive core way in Horizon for backwards compatibility, but it isn't how we want Horizon to be run.

We should run Horizon in quickstart the way we recommend others to run it, by default.

We should remove the option to choose which way to run Horizon because having two ways to run a thing makes maintaining the image harder.

@leighmcculloch leighmcculloch marked this pull request as ready for review September 23, 2023 02:50
@sreuland
Copy link
Contributor

sreuland commented Oct 2, 2023

nice, horizon's STELLAR_CORE_DATABASE_URL has been deprecated, so, it was time, STELLAR_CORE_DATABASE_URL can be deleted from the horizon.env files also - 7e91338

@sreuland sreuland self-requested a review October 2, 2023 23:32
@sreuland
Copy link
Contributor

sreuland commented Oct 2, 2023

this could Close #490 at same time by adding bucketlist db to cfg, closely related to same horizon captive core usage - ac9088a

@leighmcculloch
Copy link
Member Author

nice, horizon's STELLAR_CORE_DATABASE_URL has been deprecated, so, it was time, STELLAR_CORE_DATABASE_URL can be deleted from the horizon.env files also - 7e91338

Addressed in f3eee72. I also moved some configs from being set dynamically in the start script into the static horizon.env files.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 2, 2023

this could Close #490 at same time by adding bucketlist db to cfg, closely related to same horizon captive core usage - ac9088a

I'd rather keep this pull request focused on doing one thing, which is removing the option to control captive. For anyone using captive-core today this change represents a non-functional change, it's 100% refactor and should result in zero side-effects. Changing how captive-core runs would be a functional change and we should keep it separate so that it's easier to tell that the non-functional change is truly non-functional.

@leighmcculloch leighmcculloch enabled auto-merge (squash) October 3, 2023 19:56
@leighmcculloch leighmcculloch merged commit 140ee88 into master Oct 3, 2023
60 checks passed
@leighmcculloch leighmcculloch deleted the remove-horizon-that-does-not-use-captive-core branch October 3, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants