-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Have BaseServiceV2 add spaces to environment variable names #2301
Description
Currently, BaseServiceV2 automatically generates environment variables based on the following piece of code:
optimism/packages/common-ts/src/base-service/base-service-v2.ts
Lines 114 to 120 in 779709f
| program.addOption( | |
| new Option(`--${optionName.toLowerCase()}`, `${optionSpec.desc}`).env( | |
| `${params.name | |
| .replace(/-/g, '_') | |
| .toUpperCase()}__${optionName.toUpperCase()}` | |
| ) | |
| ) |
Notice that the name of the variable ends with optionName.toUpperCase(). This creates a situation where environment variables look like:
MY_SERVICE_NAME__MYVARIABLENAME
However, we prefer to have variables like:
MY_SERVICE_NAME__MY_VARIABLE_NAME
I think the correct way to handle this is to turn the variable name into snake case, then make it upper case. There might be some issues later with the piece of code where we actually load these environment variables. If that does end up being a problem, we'll figure it out when we run into it:
optimism/packages/common-ts/src/base-service/base-service-v2.ts
Lines 152 to 155 in 779709f
| config.load({ | |
| env: true, | |
| argv: true, | |
| }) |
Once this is done, you will also have to update some places where the old variable style is used, for instance:
optimism/ops/docker-compose.yml
Lines 139 to 141 in 779709f
| MESSAGE_RELAYER__L1RPCPROVIDER: http://l1_chain:8545 | |
| MESSAGE_RELAYER__L2RPCPROVIDER: http://l2geth:8545 | |
| MESSAGE_RELAYER__L1WALLET: '0xdbda1821b80551c9d65939329250298aa3472ba22feea921c0cf5d620ea67b97' |
And:
The only two services that use BaseServiceV2 are the message-relayer and the replica-healthcheck. You should be able to find all places where the old environment variable style is used by searching for MESSAGE_RELAYER and REPLICA_HEALTHCHECK in the codebase.