Skip to content

Commit

Permalink
fix: allow common option both before and after run
Browse files Browse the repository at this point in the history
  • Loading branch information
tpluscode committed Aug 9, 2023
1 parent 0b508e1 commit ec47a51
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 16 deletions.
12 changes: 12 additions & 0 deletions .changeset/gentle-colts-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"barnard59": minor
---

Common CLI flags are now support both when before and after the `run` command.

For example, these two commands are now equivalent:

```shell
barnard59 run file.ttl --verbose
barnard59 --verbose run file.ttl
```
18 changes: 18 additions & 0 deletions .changeset/gorgeous-bats-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"barnard59": major
---

Monitoring flags moved before commands:

- `--enable-buffer-monitor`
- `--otel-debug`
- `--otel-metrics-exporter`
- `--otel-metrics-interval`
- `--otel-traces-exporter`

Update scripts like

```diff
-barnard59 run pipeline.ttl --enable-buffer-monitor
+barnard59 --enable-buffer-monitor run pipeline.ttl
```
12 changes: 7 additions & 5 deletions packages/cli/bin/barnard59.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'
import { BatchSpanProcessor } from '@opentelemetry/tracing'

import { Command } from 'commander'
import * as otelOptions from '../lib/cli/otelOptions.js'
import * as monitoringOptions from '../lib/cli/monitoringOptions.js'

const sdk = new NodeSDK({
// Automatic detection is disabled, see comment below
Expand Down Expand Up @@ -45,10 +45,12 @@ const onError = async err => {
// before starting the SDK and loading any other code.
const program = new Command()

program.addOption(otelOptions.tracesExporter)
program.addOption(otelOptions.metricsExporter)
program.addOption(otelOptions.metricsInterval)
program.addOption(otelOptions.debug)
program
.addOption(monitoringOptions.enableBufferMonitor)
.addOption(monitoringOptions.tracesExporter)
.addOption(monitoringOptions.metricsExporter)
.addOption(monitoringOptions.metricsInterval)
.addOption(monitoringOptions.debug)

// Command#parseOptions() does not handle --help or run anything, which fits
// well for this use case. The options used here are then passed to the
Expand Down
19 changes: 12 additions & 7 deletions packages/cli/lib/cli.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
import program from 'commander'
import runAction from './cli/runAction.js'
import * as otelOptions from './cli/otelOptions.js'
import * as monitoringOptions from './cli/monitoringOptions.js'
import * as commonOptions from './cli/commonOptions.js'

program
.addOption(commonOptions.variable)
.addOption(commonOptions.variableAll)
.addOption(commonOptions.verbose)
.addOption(commonOptions.enableBufferMonitor)
.addOption(otelOptions.debug)
.addOption(otelOptions.metricsExporter)
.addOption(otelOptions.metricsInterval)
.addOption(otelOptions.tracesExporter)
.addOption(monitoringOptions.enableBufferMonitor)
.addOption(monitoringOptions.debug)
.addOption(monitoringOptions.metricsExporter)
.addOption(monitoringOptions.metricsInterval)
.addOption(monitoringOptions.tracesExporter)

program
const runCommand = program
.command('run <filename>')
.option('--output [filename]', 'output file', '-')
.option('--pipeline [iri]', 'IRI of the pipeline description')
.action(runAction)

export default async function () {
runCommand
.addOption(commonOptions.variable)
.addOption(commonOptions.variableAll)
.addOption(commonOptions.verbose)

await program.parseAsync(process.argv)
}
2 changes: 0 additions & 2 deletions packages/cli/lib/cli/commonOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,3 @@ export const variableAll = new Option('--variable-all', 'Import all environment
export const verbose = new Option('-v, --verbose', 'enable diagnostic console output')
.default(0)
.argParser((v, total) => ++total)

export const enableBufferMonitor = new Option('--enable-buffer-monitor', 'enable histogram of buffer usage')
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ export const metricsInterval = new Option('--otel-metrics-interval <milliseconds
export const debug = new Option('--otel-debug <level>', 'Enable OpenTelemetry console diagnostic output')
.choices([...Object.keys(DiagLogLevel)].filter(opt => isNaN(Number.parseInt(opt, 10))))
.default('ERROR')

export const enableBufferMonitor = new Option('--enable-buffer-monitor', 'enable histogram of buffer usage')
8 changes: 7 additions & 1 deletion packages/cli/lib/cli/runAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { SpanStatusCode } from '@opentelemetry/api'
import fromStream from 'rdf-dataset-ext/fromStream.js'
import rdf from '@zazuko/env'
import fromFile from 'rdf-utils-fs/fromFile.js'
import program from 'commander'
import runner from '../../runner.js'
import findPipeline from '../../findPipeline.js'
import bufferDebug from './../bufferDebug.js'
Expand All @@ -29,7 +30,12 @@ function createOutputStream(output) {
return createWriteStream(output)
}

export default async function (filename, { output, pipeline: iri, variable: variables, variableAll, verbose, enableBufferMonitor } = {}) {
export default async function (filename, options = {}) {
const { output, pipeline: iri, variable: variables, variableAll, verbose, enableBufferMonitor } = {
...program.opts(),
...options,
}

await tracer.startActiveSpan('barnard59 run', async span => {
try {
const level = ['error', 'info', 'debug'][verbose] || 'error'
Expand Down
32 changes: 31 additions & 1 deletion packages/cli/test/barnard59.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import filenamePipelineDefinition from './support/filenamePipelineDefinition.js'
const barnard59 = (new URL('../bin/barnard59.js', import.meta.url)).pathname
const cwd = new URL('..', import.meta.url).pathname

describe('barnard59', () => {
describe('barnard59', function () {
this.timeout(10000)

describe('run', () => {
it('should exit with error code 0 if there are no error while processing the pipeline', () => {
const pipelineFile = filenamePipelineDefinition('simple')
Expand Down Expand Up @@ -37,6 +39,15 @@ describe('barnard59', () => {
strictEqual((/^info: /m).test(result), true)
})

it('should log info messages if verbose flag is set before command', () => {
const pipelineFile = filenamePipelineDefinition('simple')
const command = `${barnard59} --verbose run --pipeline=http://example.org/pipeline/ ${pipelineFile}`

const result = stripAnsi(shell.exec(command, { silent: true, cwd }).stderr)

strictEqual((/^info: /m).test(result), true)
})

it('should not log debug messages if verbose flag is set', () => {
const pipelineFile = filenamePipelineDefinition('simple')
const command = `${barnard59} run --pipeline=http://example.org/pipeline/ --verbose ${pipelineFile}`
Expand Down Expand Up @@ -66,6 +77,15 @@ describe('barnard59', () => {
strictEqual(result.includes('abc: 123'), true)
})

it('should set the given variable to the given value before command', () => {
const pipelineFile = filenamePipelineDefinition('simple')
const command = `${barnard59} --variable=abc=123 run --pipeline=http://example.org/pipeline/ --verbose ${pipelineFile}`

const result = stripAnsi(shell.exec(command, { silent: true, cwd }).stderr)

strictEqual(result.includes('abc: 123'), true)
})

it('should set the given variable to the value of the environment variable with the same name', () => {
const pipelineFile = filenamePipelineDefinition('simple')
const command = `abc=123 ${barnard59} run --pipeline=http://example.org/pipeline/ --verbose ${pipelineFile} --variable=abc`
Expand All @@ -86,6 +106,16 @@ describe('barnard59', () => {
expect(result).to.match(/info:.+abc: 123/)
expect(result).to.match(/debug:.+def: 456/)
})

it('should import all environment variables before command', () => {
const pipelineFile = filenamePipelineDefinition('simple')
const command = `abc=123 def=456 ${barnard59} --variable-all run --pipeline=http://example.org/pipeline/ -vv ${pipelineFile}`

const result = stripAnsi(shell.exec(command, { silent: true }).stderr)

expect(result).to.match(/info:.+abc: 123/)
expect(result).to.match(/debug:.+def: 456/)
})
})
})

Expand Down

0 comments on commit ec47a51

Please sign in to comment.