Are all these environment variables called for? well documented?

[erights]

The usual process.env idiom uses ambient authority while our style is to pass all authority around explicitly; that is: to use OCap Discipline.

Searching just agoric-sdk for "env." I get 52 hits! Across 26 files! The unique names seem to be

PORT DEBUG AGORIC_INSTALL FAKE_CHAIN_DELAY HOST_PORT CHAIN_PORT HOME (ok at least that's an existing one) NO_FAKE_CURRENCIES SLOGFILE OTEL_EXPORTER_PROMETHEUS_HOST ROLLUP_WATCH DO_API_TOKEN DOCKER_VOLUMES NETWORK_NAME ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS AG_SETUP_COSMOS_NAME AG_SETUP_COSMOS_HOME SWINGSET_WORKER_TYPE AG_SOLO_BASEDIR SOLO_PORT NOISY AUTOBENCH_METRICS_URL MODDABLE_COMMIT_HASH MODDABLE_URL
And my own sin: LOCKDOWN_OPTIONS

We need a principled way to pass around configuration settings. This ain't it.

[dckc]

packages/agoric-cli - should be documented

• env.PORT, env.HOST_PORT, env.CHAIN_PORT seem sort of reasonable, but they should be documented; I don't see documentation in https://agoric.com/documentation/guides/agoric-cli/commands.html.
  • file doc issue: agoric-cli documentation missing $FAKE_CHAIN_DELAY, $CHAIN_PORT, etc. documentation#536
• env.FAKE_CHAIN_DELAY is another agoric start parameter
• env.DEBUG seems reasonable; I suspect we actually rely on it. Perhaps in some sense it should remain internal / undocumented, but since our users could trip over it, we probably owe them documentation.
• env.AGORIC_INSTALL is a solution to some yarn link stuff. Again, we owe documentation.
• process.env.HOME

I would like to see process.env passed around explicitly rather than being used ambiently. See below for an example.

[dckc]

packages/deployment - pass

The following come from packages/deployment. As this is more internal operations than customer-visible product, and we aim to be decreasingly involved in validator deployment, @erights and I agreed to leave this package out of scope.

Meanwhile, note that process.environment is passed explicitly around in this package, not used ambiently.
https://github.com/Agoric/agoric-sdk/blob/master/packages/deployment/src/entrypoint.js

• env.DO_API_TOKEN
• env.DOCKER_VOLUMES
• env.NETWORK_NAME
• env.ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS
• env.AG_SETUP_COSMOS_NAME

[dckc]

anylogger - process.env.DEBUG

packages/cosmic-swingset/src/anylogger-agoric.js, like many logging frameworks, uses process.env.DEBUG ambiently. I'm sympathetic to an exception to ocap discipline for logging, but it does worry me a bit.

#1318 is still open; that's where I mentally track this one.

[dckc]

cosmic-swingset: process.env passed explicitly per ocap discipline

• env.NO_FAKE_CURRENCIES
  • darn it; sim-chain.js uses process.env.NO_FAKE_CURRENCIES ambiently
• env.OTEL_EXPORTER_PROMETHEUS_HOST (following open telemetry conventions)
• env.OTEL_EXPORTER_PROMETHEUS_PORT

[dckc]

dapp-svelte: rollup

It doesn't seem cost-effective to go to any heroics to avoid process.env.ROLLUP_WATCH in rollup.config.js

[dckc]

install-ses: process.env.LOCKDOWN_OPTIONS

somewhat like the process.env.DEBUG trade-off.

[dckc]

swingset, xsnap: XS transition, debugging

• env.SWINGSET_WORKER_TYPE allows us to run whole test suites either with our without XS. For history, see feat(swingset): boot xs-worker vats using snapshots from snapstore #2370
• debug: !!env.XSNAP_DEBUG - xsnap debug / release switch for integration testing
• env.XSNAP_DEBUG_RR is for time-travel debugging with rr, which has not proved sufficiently robust that we rely on it. Perhaps it should be removed.

[dckc]

packages/solo

• process.env.AG_SOLO_BASEDIR - for integration testing, I think.
• defaultManagerType: process.env.SWINGSET_WORKER_TYPE - as in swingset above
• process.env.DEBUG as above
• process.env.NOISY capTP config

[dckc]

swingset-runner scripts

• process.env.AUTOBENCH_METRICS_URL is in packages/swingset-runner/src/push-metrics.js, which ends with process.exit(curlCp.status);, so it's clearly a cli script, not a module that anyone might import.

[dckc]

xsnap build script

• process.env.MODDABLE_COMMIT_HASH, process.env.MODDABLE_URL are in a top-level "run this now" expression in src/build.js, which makes it clear that this is a cli script and not a module that anyone might import.

[dckc]

scripts vs. modules

@erights and I agreed that using process.env is much more reasonable in scripts than modules. As discussed in #2160 and https://github.com/Agoric/agoric-sdk/wiki/OCap-Discipline , scripts should start with a #! line. /* global process */ is still appropriate, but a long /* global ... */ list in a #! script is to be expected, while in a module, we aim to avoid ambient authority.

To use environment variables in modules, env should be passed in as an explict authority argument as usual per ocap discipline. @warner @michaelfig and I have done some work in that direction, but some exceptions remain. @erights and I propose that remaining exceptions represent technical-debt . @warner @michaelfig I'd appreciate it if you would confirm with 👍 (unless you would prefer to argue otherwise).

exceptions for env.DEBUG logging, .LOCKDOWN_OPTIONS

LOCKDOWN_OPTIONS is cost-effective because js modules don't have parameters.

DEBUG logging is cost-effective because... well... we need to debug.

[kriskowal]

install-ses: process.env.LOCKDOWN_OPTIONS

somewhat like the process.env.DEBUG trade-off.

I would like to change this to a range of LOCKDOWN_X_TAMING variables and/or respect NODE_ENV and/or ENV for a bank of production vs deployment (default) defaults. Alternately, I would buy an environment variable that is a the path to a lockdown.json config file or module, e.g., LOCKDOWN_CONFIG=@agoric-ses/install/development.json.

Inlining a JSON file into an environment file is strictly worse than referring to a JSON file by path, capturing by path, or selecting a path based on another environment variable.

I fully expect we will one day use an environment variable to set up daemons with a shared causal graph inspection server in development or production modes, and we wouldn’t use a config file for those.

[kriskowal]

swingset, xsnap: XS transition, debugging

• env.SWINGSET_WORKER_TYPE allows us to run whole test suites either with our without XS. For history, see feat(swingset): boot xs-worker vats using snapshots from snapstore #2370
• debug: !!env.XSNAP_DEBUG - xsnap debug / release switch for integration testing
• env.XSNAP_DEBUG_RR is for time-travel debugging with rr, which has not proved sufficiently robust that we rely on it. Perhaps it should be removed.

I’d like to float the idea of consolidating XSNAP_DEBUG under DEBUG=xsnap:....

[kriskowal]

packages/solo

• process.env.AG_SOLO_BASEDIR - for integration testing, I think.
• defaultManagerType: process.env.SWINGSET_WORKER_TYPE - as in swingset above
• process.env.DEBUG as above
• process.env.NOISY capTP config

NOISY should be prefaced with a namespace, e.g., CAPTP_LOG_LEVEL=verbose or merely fold into DEBUG=captp.

[michaelfig]

NOISY should be prefaced with a namespace, e.g., CAPTP_LOG_LEVEL=verbose or merely fold into DEBUG=captp.

Ah yes, NOISY. Actually, it has very little to do with captp and everything to do with test fixtures. So, maybe DEBUG=test?

[warner]

I’d like to float the idea of consolidating XSNAP_DEBUG under DEBUG=xsnap:....

As long as the syntax is discoverable. I've lost count of of the number of times I've been thwarted by some all-singing-all-dancing debug-level-specifying DSL keyed by such a short string (and encapsulated in an equally-sonorous logging management library) that I couldn't grep or google for the specification/docs, and whose syntax was dense and clever and elegant and completely impossible to experiment your way into learning. And my workaround is always to ignore it and just add more console.logs. q.v. #1318 .

[dckc]

@michaelfig you offered to make some proposal about what to do here, right?

[michaelfig]

@michaelfig you offered to make some proposal about what to do here, right?

My only suggestions were to change $NOISY to DEBUG=test and also to use underbar separated prefixes for the subsystem that the environment variable affects.

[erights]

#3525 locally introduces a library abstraction, environment-options, for managing static configuration parameters through apparent environment variables. In #3525 this applied only to a first pressing use case, ALLOW_IMPLICIT_REMOTABLES. But if we're happy with it, I intend to migrate it to Endo for more general reuse. Just saying.

[erights]

Unfortunately yes.

[erights]

Open!

On Sun, Nov 13, 2022 at 12:45 AM abebeos ***@***.***> wrote: Ok, @erights <https://github.com/erights> , I believe I can help you to "unblock" some issues (this one and others) simply by altering a little bit the way you process issues (both, as individuals and as a team). In the end, it's just a few suggestions, immediately actionable. Will not take many time on your side, I just need to know if you're open to it. — Reply to this email directly, view it on GitHub <#3581 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACC3TBJOMC7FYWSKI7ZUSTWICTDLANCNFSM5BNVGZBA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Cheers, --MarkM

[erights]

See endojs/endo#1266

See #7410 (review)