Add context option #60
Conversation
|
@eranhare Hi and thank you for replying! I'm not sure I understand your comment. Maybe giving an example would help. In my pet project I use your healtcheck to monitor sql.DB. So I use your healthcheck, Ping, to be exact. Inside of the database/sql code I have a hook which uses context. Currently there is no easy to provide context to the healthcheck, so empty TODO context is always passed to db driver methods. The option added in PR allows to overwrite that TODO context with any other context, which should be used inside checks. |
|
Wrong email…
…On Thu, 24 Oct 2024 at 23:10 Gleb Beloded ***@***.***> wrote:
@eranhare <https://github.com/eranhare> Hi and thank you for replying!
I'm not sure I understand your comment.
I didn't add a new field. I added a way to provide arbitrary context
instead of context.TODO() which is created here
<https://github.com/AppsFlyer/go-sundheit/blob/master/health.go#L35>.
Maybe giving an example would help.
In my pet project I use your healtcheck to monitor sql.DB. So I use your
healthcheck, Ping
<https://github.com/AppsFlyer/go-sundheit/blob/master/checks/ping.go#L15>,
to be exact.
Inside of the database/sql code I have a hook
<https://github.com/jackc/pgx/blob/master/tracelog/tracelog.go#L53> which
uses context. Currently there is no easy to provide context to the
healthcheck, so empty TODO context is always passed to db driver methods.
The option added in PR allows to overwrite that TODO context with any
other context, which should be used inside checks.
—
Reply to this email directly, view it on GitHub
<#60 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APIMPQYVRAMI6ID5SV6KZALZ5FH3BAVCNFSM6AAAAABQRMC7Y6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZWGI2TANZVGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@GlebBeloded I apologize, I reviewed this from my phone, and missed the point that I was later using the context to pass into health check tasks, and I intended to do exactly what you did but forgot about it... Thanks, LGTM |
|
@dmarkhas can you approve and merge? |
|
I'll review it later today, thanks for the heads up 🙂 |
|
@GlebBeloded can you please add a godoc comment that explains the purpose of the new function? |
|
@dmarkhas added a comment! |
Very useful for timeouts, context loggers and such