atlasexec/migrate-lint: added --context and handle exit status code #20
Conversation
This change adds --context to MigrateLint. It's only useful in combination with Web=true, which exposed a new edge case where Atlas returns status code 1 when linting fails. Indication of linting failure can be observed using the summary when using MigrateLint, but when using Web=true, there is no summary, only a URL. So this PR also exposes the status code of Migrate Lint when runnning from MigrateLintError
| @@ -384,7 +400,7 @@ func (c *Client) Version(ctx context.Context) (*Version, error) { | |||
| } | |||
|
|
|||
| // runCommand runs the given command and returns its output. | |||
| func (c *Client) runCommand(ctx context.Context, args []string, vs ...validator) (io.Reader, error) { | |||
| func (c *Client) runCommand(ctx context.Context, args []string, vs ...validator) (io.Reader, int, error) { | |||
There was a problem hiding this comment.
Error are values: https://go.dev/blog/errors-are-values
Define one and use it with errors.Is
There was a problem hiding this comment.
For example:
if stderr > 0, returns its content. Otherwise, return if exit-code <> 1, return errExitCode = errors.New("atlasexec: atlas command exited code 1"). Then, catch that in the caller and handle it accordingly.
| } | ||
|
|
||
| // ErrLint is returned by MigrateLintError when linting errors are detected | ||
| var ErrLint = errors.New("atlasexec: lint errors exist") |
There was a problem hiding this comment.
No need for this error. Compare the text instead.
Do you plan to consume this error somewhere? Maybe, to keep this error generic for other purposes, we can simply return exec.ExitError
| // MigrateLintError runs the 'migrate lint' command, the output is written to params.Writer | ||
| func (c *Client) MigrateLintError(ctx context.Context, params *MigrateLintParams) error { | ||
| r, err := c.runCommand(ctx, lintArgs(params)) | ||
| r, exitCode, err := c.runCommand(ctx, lintArgs(params)) |
There was a problem hiding this comment.
See my previous comment, and use errors.Is here.
| if err != nil { | ||
| return err | ||
| } | ||
| if exitCode > 1 { |
There was a problem hiding this comment.
exit code of 0 is considered non-error, or am I missing something?
This change adds --context to MigrateLint.
It's only useful in combination with Web=true, which exposed a new edge case where Atlas returns status code 1 when linting fails.
Indication of linting failure can be observed using the summary when using MigrateLint, but when using Web=true, there is no summary, only a URL.
So this PR also exposes the status code of Migrate Lint when runnning from MigrateLintError