OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
10 stars 9 forks source link

Log levels have to specifically be at the end of a command #190

Closed stuartc closed 1 year ago

stuartc commented 1 year ago

It appears that the --log flag considers install as a log level in this example.

Perhaps the list of log levels should be either singular or comma separated. It must be at the end of the command in order to work.

We should either document this in the --help description or make it non-greedy.

npm_config_prefix=./priv/openfn ./priv/openfn/lib/node_modules/@openfn/cli/dist/index.js --log debug install -a salesforce
file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/chunk-XYZNU5CH.js:81
        throw new Error(ERROR_MESSAGE_LOG_LEVEL);
              ^

Error: Unknown log level. Valid levels are none, debug, info and default.
    at file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/chunk-XYZNU5CH.js:81:15
    at Array.forEach (<anonymous>)
    at ensureLogOpts (file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/chunk-XYZNU5CH.js:61:14)
    at maybeEnsureOpts (file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/process/runner.js:752:92)
    at parse (file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/process/runner.js:754:16)
    at process.<anonymous> (file:///home/stuart/Sourcecode/lightning/priv/openfn/lib/node_modules/@openfn/cli/dist/process/runner.js:818:5)
    at process.emit (node:events:513:28)
    at emit (node:internal/child_process:937:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)

Node.js v18.12.0
josephjclark commented 1 year ago

Ah, good point.

It's because I've over engineered log to support stuff like --log compiler=debug runtime=none. Which is useful but only in niches.

I have been thinking for ages about just supporting --debug --info flags, which is really all you want to do 90% of the time and adding --log is annoying

I could probably make the command parser much smarter too to avoid this sort of thing

stuartc commented 1 year ago

Ah interesting, that is super cool... perhaps a less sexy but equally flexible way is to make it like: --log compiler=debug,runtime=none... Just to make the whole space delimited greedy args thing a non-issue?

josephjclark commented 1 year ago

Yes, that's absolutely an option! It's ok for the log syntax to be a bit dogmatic because it'll rarely be used

josephjclark commented 1 year ago

While I'm in the area - the error if you pass a bad log level isn't very nice. We should process.exit as we have started doing with other errors to avoid the stacktrace. image

josephjclark commented 1 year ago

The error thing should actually be fixed in #350 :ok_hand: