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

Implement optional support for axum 0.8's Listener trait #61

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

kanpov
Copy link
Contributor

@kanpov kanpov commented Feb 9, 2025

Everything about this PR is mostly obvious, with the feature being named axum08 similarly to the tonic-related compat features. There is one exception, however, that being the accept error handling behavior: the Listener's accept definition is infallible and states:

/// If the underlying accept call can return an error, this function must
/// take care of logging and retrying.

I used the approach axum developers themselves used for implementing Listener for TCP and UDS listeners, more info in the handle_accept_error method that is almost a direct copy-paste from axum's internals. My assumption is that if it's considered "good enough" by axum, it should be for tokio-vsock, but I might be wrong.

@jalil-salame
Copy link
Contributor

The commit message check requires this commit to have a body, you can simply summarize the issue (that this is required due to the orphan rule) and it should be fine c:

Thanks for this! Would've loved to have access to this 2 years ago!

Doing this is required due to how the orphan rule works in Rust

Signed-off-by: kanpov <karpovanton729@gmail.com>
@kanpov
Copy link
Contributor Author

kanpov commented Feb 10, 2025

@jalil-salame Fixed and squashed!

@jalil-salame jalil-salame self-requested a review February 10, 2025 08:03
@jalil-salame jalil-salame requested a review from qwandor February 10, 2025 08:04
@jalil-salame jalil-salame merged commit 90d41fa into rust-vsock:master Feb 12, 2025
3 checks passed
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.

3 participants