Skip to content

Define a Drain::flush method #349

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Techcable
Copy link
Member

@Techcable Techcable commented Aug 9, 2025

This is simpler than the API in #332, but requires more work by slog_async to implement it.

See the implementation in slog-rs/async#39

All other slog libraries have a trivial implementation of this, so I will wait to add those those until (and if) this is accepted.

This is simpler than the API in slog-rs#332 ,
but requires more work by slog_async to implement it.
@Techcable Techcable force-pushed the feature/simple-flush-method branch from 5eda5fd to 901137d Compare August 9, 2025 21:36
Techcable added a commit to Techcable/slog-json that referenced this pull request Aug 9, 2025
Requires slog-rs/slog#349 to be accepted first.
Techcable added a commit to Techcable/slog-json that referenced this pull request Aug 9, 2025
Requires slog-rs/slog#349 to be accepted first.
Techcable added a commit to Techcable/slog-json that referenced this pull request Aug 9, 2025
Pinned to the branch used in slog-rs/slog#349, so that must be accepted first.
The 'pretty' example is pinned to use the branch in slog-rs/async#39,
so that must also be accepted first.
@dpc
Copy link
Collaborator

dpc commented Aug 11, 2025

The goal here is to propagate any errors from the inside of the logging?

The interface doesn't say anything about calling it multiple times. Probably good idea to say something, even if "is unspecified".

@Techcable
Copy link
Member Author

I am considering adjusting the flush method to accept a FlushRequest where

enum FlushRequest {
    Block,
    BlockUntil {
        timeout: Option<Duration>,
    },
    NonBlocking,
}

Blocking with a timeout or nonblocking flush only makes sense with slog_async, so I'm not sure if it is worthwhile.

The goal here is to propagate any errors from the inside of the logging?

The goal is to flush buffered streams. Setting up a serde_json::Json with the default settings does not flush output by default, potentially losing messages if the program exits. A similar problem can happen with slog_async, and this is easier to use than an AsyncGuard.

The interface doesn't say anything about calling it multiple times. Probably good idea to say something, even if "is unspecified".

Flushing should be idempotent. I will add this to the docs.

@Techcable Techcable marked this pull request as draft August 13, 2025 03:33
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.

2 participants