Skip to content

Update DAB CLI to return appropriate exit codes based upon exit reason#2084

Merged
seantleonard merged 13 commits intomainfrom
dev/sean/cli_validReturnCodes
Mar 29, 2024
Merged

Update DAB CLI to return appropriate exit codes based upon exit reason#2084
seantleonard merged 13 commits intomainfrom
dev/sean/cli_validReturnCodes

Conversation

@seantleonard
Copy link
Copy Markdown
Contributor

Why make this change?

  • Closes [Bug]: Dab start command exited with code 0 #1777
    • Certain failures that occurred within CLI execution resulted in the CLI returning exit code 0 for success, even though there were failures.
      • For example, dab start {options} may contain fully valid values, but when used as arguments to start the DAB engine process within CLI, engine startup fails. The failure didn't get reflected as error code -1.
      • For example, dab init {options} may reference a file that already exists in the path that the CLI uses to read/write config files, this results in an error that is not properly reflected in the exit code.

What is this change?

  • Any failures that occur in CLI usage will result in the proper exit codes:
    • 0 success
    • -1 failure
  • The change updates the usage of CommandLineParser. Instead of swallowing exceptions and the status of isSuccess within the options.Handler() methods, the handler methods all return the int error code now.
  • e.g. the following now captures the return codes after command line parsing.
parser.ParseArguments<InitOptions, AddOptions, UpdateOptions, StartOptions, ValidateOptions, ExportOptions, AddTelemetryOptions>(args)
                .MapResult(

How was this tested?

  • Integration Tests

@seantleonard
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Looking for const codes to indicate success or error

Comment thread src/Cli/Exporter.cs
Comment thread src/Cli/Exporter.cs
Comment thread src/Cli/Commands/InitOptions.cs Outdated
@seantleonard seantleonard requested a review from Aniruddh25 March 12, 2024 02:14
Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread src/Cli.Tests/EndToEndTests.cs Outdated
Comment thread src/Cli/DabCliParserErrorHandler.cs Outdated
Copy link
Copy Markdown
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

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

Looks good, just a nit.

…ults in error tracking. also added "actual" named param to assert test cases for consistency since "expected" was included.
@seantleonard
Copy link
Copy Markdown
Contributor Author

/azp run

@seantleonard seantleonard enabled auto-merge (squash) March 29, 2024 17:04
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@seantleonard seantleonard merged commit 0fbf7cf into main Mar 29, 2024
@seantleonard seantleonard deleted the dev/sean/cli_validReturnCodes branch March 29, 2024 17:31
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.

[Bug]: Dab start command exited with code 0

4 participants