Skip to content

fix(cli): exit non-zero from unimplemented commands - #390

Merged
jbonofre merged 3 commits into
apache:mainfrom
kayemkim:fix/cli-stubs-exit-nonzero
Sep 24, 2026
Merged

jbonofre merged 3 commits into
apache:mainfrom
kayemkim:fix/cli-stubs-exit-nonzero

Conversation

@kayemkim

Copy link
Copy Markdown
Contributor

Summary

On main, ossie validate prints not yet implemented and exits 0, and so do convert, plugin install and plugin remove:

$ ossie validate model.yaml; echo $?
not yet implemented
0

In a CI step like ossie validate model.yaml && deploy that is indistinguishable from a pass. #266 reports it for validate, and #345 lists all four stubs in its closing note as a likely separate issue.

Each stub now returns an error naming the command, which cobra prints on stderr and main turns into exit 1:

$ ossie validate model.yaml; echo $?
Error: ossie validate is not yet implemented
1

Usage is silenced on this path since the arguments were not the problem; --help and -h still print help and exit 0. The test drives the four invocations through rootCmd.Execute(), which is what main maps to the exit code, and fails on current main.

This overlaps with #349 in two places: the runPluginInstall body, and three cases in its TestArgumentValidation (plugin install foo, plugin install --all, convert --from x --input y) that expect no error and would now get the not-implemented one. Both are mine, so whichever lands second I will rebase, flipping those three to expect the error.

Related Issues

Addresses the first item of #266 and the closing note in #345.

Checklist

Specification

  • Spec changes are included in core-spec/ and follow the existing structure
  • Spec changes have been discussed on the mailing list or in a linked issue
  • Breaking changes to the spec are clearly called out in the summary

Ontology

  • Ontology changes in ontology/ are consistent with spec changes
  • New or modified terms are defined and documented

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory

Validation

  • Validation rules in validation/ are updated if the spec changed
  • New validation cases are covered by tests

Documentation

  • docs/ is updated to reflect any user-facing changes
  • New features or behaviors are documented with examples where appropriate
  • CONTRIBUTING.md is updated if the contribution process changed

Examples

  • examples/ are added or updated for any new spec constructs or converter support

Tests

  • All existing tests pass (pytest / CI green)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added without PMC/IPMC approval

khush-bhatia
khush-bhatia previously approved these changes Sep 15, 2026
@kayemkim

Copy link
Copy Markdown
Contributor Author

@khush-bhatia friendly ping, when you get a chance. The CLI CI run on this head is still waiting for workflow approval, which is why it never went green. I've held off rebasing so your review stays in place.

@jbonofre
jbonofre self-requested a review September 24, 2026 14:32
@jbonofre

Copy link
Copy Markdown
Member

@kayemkim I'm doing the review.

kayemkim and others added 3 commits September 24, 2026 17:22
validate, convert, plugin install and plugin remove printed
"not yet implemented" to stdout and returned nil, so they exited 0
while doing nothing. In a pipeline such as
`ossie validate model.yaml && deploy` that reads as a pass.

Each stub now returns an error naming the command, which cobra
reports on stderr and main turns into exit 1. Usage is silenced
since the arguments were not the problem.

Generated-by: Claude Code
The convert case leaves --from and --input set on the shared rootCmd,
so a later test running convert without flags would skip the required
flag check.

Generated-by: Claude Code

@jbonofre jbonofre left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jbonofre
jbonofre merged commit 3ccd934 into apache:main Sep 24, 2026
1 check passed
@kayemkim
kayemkim deleted the fix/cli-stubs-exit-nonzero branch October 10, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants