Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dbt errors #2895

Merged
merged 4 commits into from Jan 31, 2022
Merged

Improve dbt errors #2895

merged 4 commits into from Jan 31, 2022

Conversation

guy-har
Copy link
Contributor

@guy-har guy-har commented Jan 30, 2022

No description provided.

}
return copyDBWithTransformLocation(ctx, fromClient, toClient, fromDB, toDB, transformLocation)
}

func copyDBWithTransformLocation(ctx context.Context, fromClient, toClient Client, fromDB string, toDB string, transformLocation func(location string) (string, error)) error {
schema, err := fromClient.GetDatabase(ctx, fromDB)
if err != nil {
return err
return fmt.Errorf("failed to get database on copy: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

would add the fromDB to the error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return toClient.CreateDatabase(ctx, schema)
err = toClient.CreateDatabase(ctx, schema)
if err != nil {
return fmt.Errorf("failed to create database with name %s and location %s :%w", schema.Name, schema.LocationURI, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

  • fix the spaces
  • prefer to use '%s' to quote names/values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return ReplaceBranchName(location, toBranch)
transformedLocation, err := ReplaceBranchName(location, toBranch)
if err != nil {
return "", fmt.Errorf("failed to replace branch name with location: %s and branch: %s : %w", location, toBranch, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

optional:

Suggested change
return "", fmt.Errorf("failed to replace branch name with location: %s and branch: %s : %w", location, toBranch, err)
return "", fmt.Errorf("failed to replace branch name with location '%s' and branch '%s': %w", location, toBranch, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return ReplaceBranchName(location, toBranch)
transformedLocation, err := ReplaceBranchName(location, toBranch)
if err != nil {
return "", fmt.Errorf("failed to replace branch name with location: %s and branch: %s : %w", location, toBranch, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return "", fmt.Errorf("failed to replace branch name with location: %s and branch: %s : %w", location, toBranch, err)
return "", fmt.Errorf("failed to replace branch name with location '%s' and branch '%s': %w", location, toBranch, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -168,7 +169,7 @@ func dbtDebug(projectRoot string) string {
dbtCmd.Dir = projectRoot
output, err := dbtCmd.Output()
if err != nil {
DieErr(err)
DieFmt("failed to run dbt debug with output:\n %s\n and error: %s", output, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DieFmt("failed to run dbt debug with output:\n %s\n and error: %s", output, err)
DieFmt("Failed to run dbt debug with output:\n%s\nError: %s", output, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -146,7 +147,7 @@ func getDbtTables(projectRoot string) ([]model, error) {
dbtCmd.Dir = projectRoot
output, err := dbtCmd.Output()
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to run dbt debug with output:\n %s\n and error: %s", output, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about this one - it looks like an error that goes to the output not something that failed - it capture the output.
is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, when dbt debug returns an error, the information for the error is printed to the standard outpout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it, now, in case of error: the output is first printed as output and the error as error

@guy-har guy-har marked this pull request as ready for review January 31, 2022 08:20
@guy-har guy-har requested a review from nopcoder January 31, 2022 08:20
@guy-har guy-har force-pushed the chore/improve-dbt-errors branch 2 times, most recently from c6506e6 to 82def39 Compare January 31, 2022 09:11
Copy link
Contributor

@nopcoder nopcoder left a comment

Choose a reason for hiding this comment

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

LGTM

@guy-har guy-har added the include-changelog PR description should be included in next release changelog label Jan 31, 2022
@guy-har guy-har merged commit 1f595c8 into master Jan 31, 2022
@guy-har guy-har deleted the chore/improve-dbt-errors branch January 31, 2022 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-changelog PR description should be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants