rocket-pool / smartnode-install

The install script for a Rocket Pool smart node.
GNU General Public License v3.0
129 stars 77 forks source link

Replace Prometheus envsubst variables with text/template variables #113

Closed jshufro closed 10 months ago

jshufro commented 11 months ago

So, the syntax here is two phrases.

First, (index . "FOO") when passed a settings map[string]string is the same as settings["FOO"]

Second, or x y is the same as x ? x : y

Combining them gives us the defaulting behavior we had with envsubst.

In a future change, I suggest we move away from using map[string]string and instead just pass the config struct to the template engine. Then we can write variables such as {{or .Execution.ApiPortNumber "8545"}} which is a bit more readable.

jclapis commented 11 months ago

In a future change, I suggest we move away from using map[string]string and instead just pass the config struct to the template engine. Then we can write variables such as {{or .Execution.ApiPortNumber "8545"}} which is a bit more readable.

Probably worth doing now honestly, readability does take a bit of a hit here. I'd set up https://github.com/rocket-pool/smartnode/pull/420 with the struct setup.

jshufro commented 11 months ago

In a future change, I suggest we move away from using map[string]string and instead just pass the config struct to the template engine. Then we can write variables such as {{or .Execution.ApiPortNumber "8545"}} which is a bit more readable.

Probably worth doing now honestly, readability does take a bit of a hit here. I'd set up rocket-pool/smartnode#420 with the struct setup.

I'll add commits on top that move that way and we can decide how much we want to bite off at once based on those

jshufro commented 11 months ago

@jclapis I updated this as well as https://github.com/rocket-pool/smartnode/pull/420

Note the extra complexity in smartnode. It looks like we're adding complexity but we're actually removing it- once every template is migrated, we can delete all the environment variable fudging, instead using these cfg receiver methods returning (sprintable, error), so during the transition there is more complexity, but in the steady state there should be much less.