-
Notifications
You must be signed in to change notification settings - Fork 361
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
fixed sql-error-441 #806
fixed sql-error-441 #806
Conversation
Codecov Report
@@ Coverage Diff @@
## master #806 +/- ##
==========================================
+ Coverage 41.07% 45.10% +4.02%
==========================================
Files 129 140 +11
Lines 10044 10855 +811
==========================================
+ Hits 4126 4896 +770
+ Misses 5380 5326 -54
- Partials 538 633 +95
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Please make a change on the server side, details inside. Do tag me (@ariels) on our Slack if you have any questions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! Hadn't realized it would be this short :-)
Does this fix the error message? If you tested manually and could copy+paste the new error message into the PR, that would be great and I'll approve and pull over the weekend. Otherwise I'll do these steps on Sunday.
@@ -38,7 +38,9 @@ func (c *cataloger) CreateRepository(ctx context.Context, repository string, sto | |||
|
|||
// create repository with ref to branch | |||
if _, err := tx.Exec(`INSERT INTO catalog_repositories (id,name,storage_namespace,creation_date,default_branch) | |||
VALUES ($1,$2,$3,transaction_timestamp(),$4)`, repoID, repository, storageNamespace, branchID); err != nil { | |||
VALUES ($1,$2,$3,transaction_timestamp(),$4)`, repoID, repository, storageNamespace, branchID); db.IsUniqueViolation(err) { | |||
return nil, fmt.Errorf("%s %w", repository, db.ErrAlreadyExists) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return nil, fmt.Errorf("%s %w", repository, db.ErrAlreadyExists) | |
return nil, fmt.Errorf("create repository %s: %w", repository, db.ErrAlreadyExists) |
(details the operation on the error message).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I updated the comment above with a screenshot of the new error message - 'error creating repository:' is prefixed to the message already. 🙂
Sorry if the below is unclear:
Short is good and contains fewer places for bugs to show up later. I like "short". "Short" is not the same as "easy". So thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks!
#441
New Output: