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

Change: Remove RemoteError variant from StreamingError #1325

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

drmingdrmer
Copy link
Member

@drmingdrmer drmingdrmer commented Feb 5, 2025

Changelog

Change: Remove RemoteError variant from StreamingError

StreamingError is intended to represent non-logic errors that occur
while streaming data to a remote peer. A RemoteError does not belong
in a stream RPC error and should instead be included as part of a
non-error response.

Upgrade tip:

No modifications are required for this change, as no related API
utilized the RemoteError variant.

Change: Simplify send_snapshot() error type in Chunked

The return error type of
openraft::network::snapshot_transport::Chunked::send_snapshot() is
simplified from StreamingError<_, Fatal> to StreamingError<_>. This
change is made because the remote Fatal error cannot be handled by the
sending end and should not be produced at all.

Upgrade Tip:

If Chunked is used in a RaftNetworkV2 implementation to adapt the v1
RaftNetwork, simply remove the error handling for Fatal. Otherwise,
no changes are required.


This change is Reviewable

The return error type of
`openraft::network::snapshot_transport::Chunked::send_snapshot()` is
simplified from `StreamingError<_, Fatal>` to `StreamingError<_>`. This
change is made because the remote `Fatal` error cannot be handled by the
sending end and should not be produced at all.

Upgrade Tip:

If `Chunked` is used in a `RaftNetworkV2` implementation to adapt the v1
`RaftNetwork`, simply remove the error handling for `Fatal`. Otherwise,
no changes are required.
@drmingdrmer drmingdrmer requested a review from schreter February 5, 2025 02:53
`StreamingError` is intended to represent non-logic errors that occur
while streaming data to a remote peer. A `RemoteError` does not belong
in a stream RPC error and should instead be included as part of a
non-error response.

Upgrade tip:

No modifications are required for this change, as no related API
utilized the `RemoteError` variant.
Copy link
Collaborator

@schreter schreter left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)

@drmingdrmer drmingdrmer merged commit 926bf6d into databendlabs:main Feb 5, 2025
34 checks passed
@drmingdrmer drmingdrmer deleted the 182-stream-error branch February 5, 2025 08:53
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