paritytech / substrate-archive

Blockchain Indexing Engine
GNU General Public License v3.0
196 stars 74 forks source link

Add AMQP_URL environment variable #452

Closed joaoepj closed 2 years ago

joaoepj commented 2 years ago

This PR enables setting the value of ControlConfig::task_url() via the AMQP_URL encironment variable. It relates to issue 431

cla-bot-2021[bot] commented 2 years ago

User @joaoepj, please sign the CLA here.

insipx commented 2 years ago

this is OK, but the config file will always override the environment variable. I think it would make more sense to put the check for the environment variable along with where the check occurs for env variables CHAIN_DATA_DB and DATABASE_URL here: https://github.com/paritytech/substrate-archive/blob/6e5a6e2d22d7dd28bfb4f2eb50e5f2de6045e433/substrate-archive/src/archive.rs#L350

and then replace config.control.task_url from there if it exists. this way env variable always takes precedence over the config file

joaoepj commented 2 years ago

@insipx Ops... slowly getting used to rust basics and beginning to understand something. As you suggested I checked the AMQL_URL along with other environment variables in ArchiveBuilder::build() and it also ended up with file config prevailing over env config. So I added another check in ArchiveBuilder::with_config() achieving the desired result. Seems like ArchiveBuilder::build() scenario is used to do some database recovery. I don't figured it out exactly but leave a duplicated check there anyway. Let me know what else is needed polish it up.

this is OK, but the config file will always override the environment variable. I think it would make more sense to put the check for the environment variable along with where the check occurs for env variables CHAIN_DATA_DB and DATABASE_URL here:

https://github.com/paritytech/substrate-archive/blob/6e5a6e2d22d7dd28bfb4f2eb50e5f2de6045e433/substrate-archive/src/archive.rs#L350

and then replace config.control.task_url from there if it exists. this way env variable always takes precedence over the config file

joaoepj commented 2 years ago

Fixing code style.