stellar / quickstart

Home of the stellar/quickstart docker image for development and testing
Apache License 2.0
195 stars 206 forks source link

Remove the config supporting running horizon without a captive stellar-core #498

Closed leighmcculloch closed 1 year ago

leighmcculloch commented 1 year ago

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.

sreuland commented 1 year ago

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 - https://github.com/stellar/quickstart/pull/504/commits/7e913380ebd143559feea88cb8918cce4a51e3be

sreuland commented 1 year ago

this could Close #490 at same time by adding bucketlist db to cfg, closely related to same horizon captive core usage - https://github.com/stellar/quickstart/pull/504/commits/ac9088ad645584ef745e042c62dc9862b3dd3570

leighmcculloch commented 1 year ago

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 commented 1 year ago

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.