Skip to content

Conversation

seantleonard
Copy link
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
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
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

@seantleonard seantleonard requested a review from Aniruddh25 March 12, 2024 02:14
Copy link
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.

Copy link
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
Contributor Author

/azp run

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

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