[logger] added flush to DiskLogger #1078
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
issue: #1040
What is the purpose of this PR? Is it to
If some part of the pipeline breaks, the logger may not have a chance to flush whats in the buffer. By adding flush after every .log, we can avoid it. A better solution would be to try to gracefully shutdown if something breaks, and then call .flush there, but this should be more complex.
It does increase latency by a few ms, but it shouldn't be a problem if we are only logging a few times per step:
Changelog
Just added .flush
Test plan
pytest
pre-commit install
)pytest tests
pytest tests -m integration_test