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

Support content-encoding in worker/publisher #449

Merged
merged 6 commits into from
Apr 11, 2022

Conversation

ansoncat
Copy link
Contributor

Similar to the content-encoding header in HTTP, ContentEncoding implemented here provides a way to encode/decode (compress/decompress) messages on the fly. Like original ContentType, ContentEncoding can be configured like:

Sneakers::ContentEncoding.register(
  content_encoding: 'gzip',
  decoder: ->(payload) { ActiveSupport::Gzip.decompress(payload) },
  encoder: ->(payload) { ActiveSupport::Gzip.compress(payload) },
)

When messages arrive with the given content type and content encoding set the payload will be decoded and then deserialized. Before publishing, message will be serialized and then encoded with given content_type and content_encoding. Most of code are borrowed from ContentType (#152), thus behavior of configurations and options is pretty much the same.

With combination of ContentType and ContentEncoding, we can easily deal with something like 'text/plain'+'gzip', 'application/json'+'gzip' or 'application/json'+'zstd' without any duplicated code.

@michaelklishin
Copy link
Collaborator

michaelklishin commented Apr 11, 2022

Thank you. This is a non-trivial contribution I generally would like to see more feedback on. However, this fits the kind of features Sneakers strives to provide, and complements content type support nicely. So I think this should not receive a lot of pushback :)

@michaelklishin
Copy link
Collaborator

One thing this could do on the publisher side but does not is set the message property for content encoding, which AMQP 0-9-1 has. It also has one for content type and that isn't set either, which may be why this PR does nothing about it. The field is purely informative for
applications and is not used by RabbitMQ itself in any way.

michaelklishin added a commit that referenced this pull request Apr 11, 2022
@michaelklishin michaelklishin merged commit d16b108 into jondot:master Apr 11, 2022
michaelklishin added a commit that referenced this pull request Apr 11, 2022
@ansoncat
Copy link
Contributor Author

One thing this could do on the publisher side but does not is set the message property for content encoding, which AMQP 0-9-1 has. It also has one for content type and that isn't set either, which may be why this PR does nothing about it. The field is purely informative for applications and is not used by RabbitMQ itself in any way.

Thanks for merging code. On the publisher side, properties for content encoding and content type will be set automatically by bunny because we pass them as options to Bunny::Exchange::publish. Those options will be encoded as properties in here and then here.

@michaelklishin
Copy link
Collaborator

@ansoncat then they are not "set automatically by Bunny" but rather by your code ;) which is fine, of course.

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.

None yet

2 participants