Conversation
Support for gzip, lz4 and snappy
snichme
requested review from
ThomasSarlin,
antondalgren,
baelter,
carlhoerberg and
dentarg
as code owners
|
I have had as a proud feature of this library that this is a 0 dependency library. Could we make the dependencies optional? |
| * @param properties - properties to be published | ||
| * @param [mandatory] - if the message should be returned if there's no queue to be delivered to | ||
| * @param [immediate] - if the message should be returned if it can't be delivered to a consumer immediately (not supported in RabbitMQ) | ||
| * @param options - publish options including compression settings |
There was a problem hiding this comment.
These methods should map closely to the protocol. Add compression to a high level client.
There was a problem hiding this comment.
I agree, we should build out BaseClient to have high level features like auto-reconnect, message coding, rpc client/server etc. Just like the ruby client. This is a WIP in that direction: #180
|
Nice! I agree with
IMHO we should build out BaseClient to have high level features like auto-reconnect, message coding, rpc client/server etc. Just like the ruby client. This is a WIP in that direction: #180 |
They are, since they are specified as |
@baelter sounds good, maybe combine this PR into #180 or at least base this branch on your |
| case "zstd": | ||
| return await this.loadZstd() |
There was a problem hiding this comment.
zstd is not available.
| if (!AMQPMessage.COMPRESSION_ENCODINGS.includes(encoding)) { | ||
| // Unknown encoding, return raw body | ||
| return this.body | ||
| } | ||
|
|
||
| const codec = compressionRegistry.getCodecSync(encoding) | ||
| if (!codec) { | ||
| throw new CompressionError( | ||
| `Cannot decompress message: codec '${encoding}' is not loaded. Ensure the codec was used for compression first.`, | ||
| encoding as "gzip" | "lz4" | "snappy" | "zstd", | ||
| ) | ||
| } |
There was a problem hiding this comment.
For this, we can use type assertions methods so we don't need to do a typecast later.
Something along this lines:
function isKnownCompressionAlgorithm(algorithm: string): algorithm is CompressionAlgorithm {
return COMPRESSION_ENCODINGS.includes(algorithm)
}There was a problem hiding this comment.
Personally I don't see why we would bother handling the null type for missing codecs. Might as well just throw the compression error straight up in here wherever we return null?
| declare module "@mongodb-js/zstd" { | ||
| export function compress(data: Buffer): Promise<Buffer> | ||
| export function decompress(data: Buffer): Promise<Buffer> | ||
| } |
There was a problem hiding this comment.
| declare module "@mongodb-js/zstd" { | |
| export function compress(data: Buffer): Promise<Buffer> | |
| export function decompress(data: Buffer): Promise<Buffer> | |
| } |
We don't use this zstd encoding anyway
| if (!codec) { | ||
| console.log(`Skipping ${algorithm} test - codec not available`) | ||
| expect(true).toBe(true) // Ensure at least one assertion | ||
| return | ||
| } |
There was a problem hiding this comment.
Shouldn't we expect all these algorithms to work?
|
closing in favor of #192 |
4 participants
Adds support for the producer to compress the payload by specifying compression algorithm, supported algos are gzip, lz4 and snappy. Looked at adding zstd but didn't find any packages with a synchronous API.
The consumer automatically picks up how the payload is compressed and decompresses it.
Each package is optional and it will be lazy loaded if needed.