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

Move temporary error logs to lower level than WARN #897

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

sergerad
Copy link
Contributor

@sergerad sergerad commented Jan 12, 2025

Closes #896

Refactors logging around some PipelineErrorKind::TemporaryErrors to be lower level than WARN.

@sergerad sergerad marked this pull request as draft January 12, 2025 08:20
@sergerad sergerad changed the title Move temporary error logs to debug instead of warn Move temporary error logs to lower severity than WARN Jan 12, 2025
@sergerad sergerad changed the title Move temporary error logs to lower severity than WARN Move temporary error logs to lower level than WARN Jan 12, 2025
@sergerad
Copy link
Contributor Author

@clabby My changes have captured the lines that you have shared in image attached to the issue.

Just wanted to check whether we wanted to handle the general case of ::Temporary(_) with a trace log:

if let PipelineErrorKind::Temporary(PipelineError::Eof) = err {

And whether we want to consider any other temporary errors here:

if let PipelineErrorKind::Temporary(PipelineError::Eof) = e {

and

if !matches!(e, PipelineErrorKind::Temporary(PipelineError::Eof)) {

@sergerad sergerad marked this pull request as ready for review January 12, 2025 19:59
@clabby
Copy link
Collaborator

clabby commented Jan 13, 2025

@clabby My changes have captured the lines that you have shared in image attached to the issue.

Just wanted to check whether we wanted to handle the general case of ::Temporary(_) with a trace log:

[...]

Yeah, this looks good to me. Thank you for the change!

@clabby clabby added this pull request to the merge queue Jan 13, 2025
Merged via the queue into op-rs:main with commit b8b0528 Jan 13, 2025
13 of 14 checks passed
@github-actions github-actions bot mentioned this pull request Jan 13, 2025
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.

chore(derive): Remove redundant warning logs
2 participants