Skip to content

contrib/database/sql: Add WithErrorCheck options#1315

Merged
ajgajg1134 merged 14 commits into
DataDog:mainfrom
soh335:v1-contrib-db-sql-error-with-check
Aug 29, 2022
Merged

contrib/database/sql: Add WithErrorCheck options#1315
ajgajg1134 merged 14 commits into
DataDog:mainfrom
soh335:v1-contrib-db-sql-error-with-check

Conversation

@soh335

@soh335 soh335 commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread contrib/database/sql/conn.go Outdated
Comment thread contrib/database/sql/conn_test.go Outdated
Comment thread contrib/database/sql/conn_test.go Outdated
@soh335
soh335 requested a review from gbbr June 2, 2022 08:02
Comment thread contrib/database/sql/conn.go Outdated
Comment thread contrib/database/sql/conn_test.go
@soh335
soh335 requested a review from gbbr June 2, 2022 09:05
Comment thread contrib/database/sql/sql.go Outdated
@soh335
soh335 requested a review from gbbr June 2, 2022 12:45

@gbbr gbbr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, last comment, promise! 🤞🏻

Comment thread contrib/database/sql/conn.go Outdated
@soh335
soh335 requested a review from gbbr June 2, 2022 12:51
Comment thread contrib/database/sql/conn.go
@soh335
soh335 requested a review from gbbr June 2, 2022 14:49
gbbr
gbbr previously approved these changes Jun 3, 2022

@gbbr gbbr left a comment

Copy link
Copy Markdown
Contributor

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

soh335 commented Aug 8, 2022

Copy link
Copy Markdown
Contributor Author

how about ? @gbbr @katiehockman

gbbr
gbbr previously approved these changes Aug 8, 2022

@gbbr gbbr left a comment

Copy link
Copy Markdown
Contributor

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 gbbr and katiehockman August 24, 2022 03:13
@soh335

soh335 commented Aug 24, 2022

Copy link
Copy Markdown
Contributor Author

@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

soh335 commented Aug 30, 2022

Copy link
Copy Markdown
Contributor Author

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