Skip to content

add chunked mixed encoding support and deprecate direct decode with pest parser - #2

Open
loulecrivain wants to merge 8 commits into
mainfrom
feat/chunked-mixed-encoding
Open

loulecrivain wants to merge 8 commits into
mainfrom
feat/chunked-mixed-encoding

Conversation

@loulecrivain

Copy link
Copy Markdown
Collaborator

No description provided.

@loulecrivain

Copy link
Copy Markdown
Collaborator Author

cc @yuyuyureka would appreciate your opinion / review on the skeleton implementation, as it is my first time building stream adapters. also on the general okayness of the implem.

given we need it to be retry-able by the client in case of error in stream.

@yuyuyureka yuyuyureka left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The general approach of the regex delimiter codec is good. There's not much code to review inside this PR (only ~50L). Does this compile with various feature flag combination like without async-streaming feature / --no-default-features? Does the re-delimiter-codec need to be an optional dependency only for the async-streaming feature?

Inside the re-delimiter-codec itself, there could be more docstrings to explain the usage and what regex to actually put into REDelimiterCodec::new() with an example of what splitting it would achieve (Should the regex match only the delimiter? or the full message? will the matched part be contained within the stream message?).

Comment thread src/streaming.rs Outdated
@loulecrivain

loulecrivain commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

Does this compile with various feature flag combination like without async-streaming feature / --no-default-features? Does the re-delimiter-codec need to be an optional dependency only for the async-streaming feature?

good catch Ill make sure to check

Inside the re-delimiter-codec itself, there could be more docstrings to explain the usage and what regex to actually put into REDelimiterCodec::new() with an example of what splitting it would achieve (Should the regex match only the delimiter? or the full message? will the matched part be contained within the stream message?).

hmmm yes do agree could use a bit more documentation

@loulecrivain

Copy link
Copy Markdown
Collaborator Author

1st point resolved

@loulecrivain

Copy link
Copy Markdown
Collaborator Author

2nd point resolved
wobcom/re-delimiter-codec@208895c

@loulecrivain
loulecrivain force-pushed the feat/chunked-mixed-encoding branch from cdf0c7d to f552c2a Compare September 21, 2026 14:02
@loulecrivain

Copy link
Copy Markdown
Collaborator Author

all review comments addressed, I'm gonna continue with the actual implementation. thanks! :)

will drop tokio_stream when the new implentation works

@loulecrivain
loulecrivain force-pushed the feat/chunked-mixed-encoding branch from 08bf69b to f918fee Compare September 21, 2026 16:36
@loulecrivain
loulecrivain force-pushed the feat/chunked-mixed-encoding branch from f918fee to 2c26837 Compare September 22, 2026 09:59
@loulecrivain
loulecrivain marked this pull request as ready for review September 22, 2026 10:03
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