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

contrib/database/sql: Add WithErrorCheck options #1315

Merged
merged 14 commits into from
Aug 29, 2022

Conversation

soh335
Copy link
Contributor

@soh335 soh335 commented Jun 1, 2022

WithErrorCheck specifies a function fn which determines whether the passed
error should be marked as an error. The fn is called whenever a database/sql operation
finishes with an error.

close #1314


Almost base code is taken from #806.
I think it is good to check err as not nil before call errCheck. User maybe don't expect to pass nil error to WithErrorCheck. How do you think ?

@soh335 soh335 requested a review from a team June 1, 2022 08:34
@soh335 soh335 marked this pull request as draft June 1, 2022 08:34
WithErrorCheck specifies a function fn which determines whether the passed
error should be marked as an error. The fn is called whenever a database/sql operation
finishes with an error.

close DataDog#1314
@soh335 soh335 force-pushed the v1-contrib-db-sql-error-with-check branch from c70478d to be59f6b Compare June 2, 2022 01:19
@soh335 soh335 marked this pull request as ready for review June 2, 2022 06:55
@gbbr gbbr added this to the 1.39.0 milestone Jun 2, 2022
contrib/database/sql/conn.go Outdated Show resolved Hide resolved
soh335 and others added 3 commits June 2, 2022 16:54
Co-authored-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
Co-authored-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
@soh335 soh335 requested a review from gbbr June 2, 2022 08:02
contrib/database/sql/conn.go Outdated Show resolved Hide resolved
contrib/database/sql/conn_test.go Show resolved Hide resolved
Co-authored-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
@soh335 soh335 requested a review from gbbr June 2, 2022 09:05
contrib/database/sql/sql.go Outdated Show resolved Hide resolved
@soh335 soh335 requested a review from gbbr June 2, 2022 12:45
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Sorry, last comment, promise! 🤞🏻

contrib/database/sql/conn.go Outdated Show resolved Hide resolved
Co-authored-by: Gabriel Aszalos <gabriel.aszalos@gmail.com>
@soh335 soh335 requested a review from gbbr June 2, 2022 12:51
@soh335 soh335 requested a review from gbbr June 2, 2022 14:49
gbbr
gbbr previously approved these changes Jun 3, 2022
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Great! LGTM! Thanks for your patience!

@soh335 soh335 requested a review from gbbr July 13, 2022 15:56
@Hellzy Hellzy modified the milestones: 1.40.0, 1.41.0 Jul 15, 2022
@soh335
Copy link
Contributor Author

soh335 commented Aug 8, 2022

how about ? @gbbr @katiehockman

gbbr
gbbr previously approved these changes Aug 8, 2022
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

LGTM! Sorry for the delay

@Hellzy Hellzy modified the milestones: 1.41.0, 1.42.0 Aug 16, 2022
@soh335 soh335 requested review from gbbr and katiehockman and removed request for katiehockman and gbbr August 24, 2022 03:13
@soh335
Copy link
Contributor Author

soh335 commented Aug 24, 2022

@gbbr @katiehockman merged main branch. please re-review it. or can i help something to merge it ?

@soh335 soh335 requested review from katiehockman and removed request for gbbr August 24, 2022 12:43
@ajgajg1134 ajgajg1134 merged commit 0778620 into DataDog:main Aug 29, 2022
@soh335
Copy link
Contributor Author

soh335 commented Aug 30, 2022

thanks !

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.

contrib/database/sql: Add WithErrorCheck option
5 participants