Skip to content

Added minimal logging #6

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

Merged
merged 3 commits into from
Jun 22, 2025
Merged

Added minimal logging #6

merged 3 commits into from
Jun 22, 2025

Conversation

frnhr
Copy link
Contributor

@frnhr frnhr commented Mar 27, 2023

Something like this for the issue #3 ?

@frnhr frnhr closed this Mar 27, 2023
@frnhr frnhr reopened this Mar 27, 2023
Copy link

@lewis-wf lewis-wf left a comment

Choose a reason for hiding this comment

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

This feels pretty essential - currently got a problem where having a logger would be handy.

Copy link

@PamelaM PamelaM left a comment

Choose a reason for hiding this comment

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

This is a good addition, but I had two suggestions that block me from approving this PR. (you could always convince me, neither of them are [bug/blocking], much less [BLOCKING] -- the latter of which I've only used twice when I ran into code that would instantly corrupt data)

@craiga
Copy link

craiga commented Jan 26, 2025

FWIW I created frnhr#1 (a PR into @frnhr's fork; creating my own PR with their work seemed like it'd be rude!) which addresses the points @PamelaM raised here.

@frnhr
Copy link
Contributor Author

frnhr commented Jan 26, 2025

Thanks @craiga !

Merged it in. I didn't have time to / need to address the suggestions yet, but they definitely look valid to me.

Copy link
Owner

@groodt groodt left a comment

Choose a reason for hiding this comment

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

LGTM

@groodt groodt merged commit d8d9e2a into groodt:main Jun 22, 2025
@cgeering cgeering mentioned this pull request Jun 24, 2025
@HugoKlepsch
Copy link
Contributor

HugoKlepsch commented Jul 9, 2025

Hey all,
This patch seems to cause a serious performance regression. I have a demo of the bug here: https://github.com/HugoKlepsch/retrying-bug-demo
Issue: #20

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.

6 participants