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

Feature: payload compression #40

Merged
merged 9 commits into from
Dec 6, 2023
Merged

Feature: payload compression #40

merged 9 commits into from
Dec 6, 2023

Conversation

mempirate
Copy link
Contributor

Closes #38

Copy link
Contributor

@merklefruit merklefruit left a comment

Choose a reason for hiding this comment

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

Nice! I left some comments. One thing that doesn't convince me 100% is the compression level option: having to manually set it on the client and server manually to the same value feels like a footgun in my opinion. wdyt?

msg-socket/src/pub/socket.rs Outdated Show resolved Hide resolved
msg-socket/src/sub/socket.rs Outdated Show resolved Hide resolved
msg-wire/src/compression/mod.rs Outdated Show resolved Hide resolved
msg-wire/src/compression/mod.rs Outdated Show resolved Hide resolved
msg-wire/src/compression/mod.rs Outdated Show resolved Hide resolved
@mempirate
Copy link
Contributor Author

@merklefruit agreed, I split up the Compressor into Compressor and Decompressor. These will be individually configurable on sockets that need them (i.e. PubSocket would have with_compressor, SubSocket with_decompressor). The decompressor does not need to know the compression level.

Copy link
Contributor

@merklefruit merklefruit left a comment

Choose a reason for hiding this comment

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

awesome! LGTM!

@mempirate mempirate merged commit 27acfde into main Dec 6, 2023
8 checks passed
@merklefruit merklefruit deleted the feature/compression branch December 20, 2023 13:29
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.

Support pubsub compression
2 participants