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

spec: Support for payload compression? #51

Open
patrys opened this issue Dec 15, 2023 · 4 comments
Open

spec: Support for payload compression? #51

patrys opened this issue Dec 15, 2023 · 4 comments
Labels

Comments

@patrys
Copy link

patrys commented Dec 15, 2023

HTTP/2 only compresses the headers, so delivering webhooks with large payloads consumes a lot of bandwidth. It would be nice if a future version of the spec automatically compressed payloads above a certain threshold and required that recipients support the resulting content-encoding.

@tasn tasn added the spec label Dec 15, 2023
@tasn
Copy link
Contributor

tasn commented Dec 15, 2023

Yeah, that sounds like a good idea. So suggest that webhook consumers support compression in the operational section?

Btw, we also recommend keeping webhook payloads small, though even at 20-40kb compression can be a boon if you're sending many messages.

@patrys
Copy link
Author

patrys commented Dec 15, 2023

My personal (Saleor Commerce) use case is that we allow users to write custom GraphQL queries for webhooks so they don't have to hit the API back immediately in response to a webhook event. Those can result in up to 1MB of JSON, which is not ideal but still better than a 100kb webhook followed by ten 100kb fetches.

@zekth
Copy link
Member

zekth commented Dec 29, 2023

What comes to my mind is should we even let the lib handle the compression by itself? Because we can have several cases:

What i'm seeing in here is the case you're picturing is the third one. However i'm not sure the lib itself should take care of it; the lib / spec are targeting the real content of it and not what the carrier transports. I'm not sure we should introduce the compression handling directly in the lib.

@tasn
Copy link
Contributor

tasn commented Jan 1, 2024

Upon further reading the original message I think I misread it. The way I read it is to include a line somewhere that it's recommended that the receiving server (or the sending client? or both?) support HTTP compression. Just a recommendation somewhere in the spec to mention that this is useful. I very much agree that we should not be doing compression in the libs, this is a transport layer concern (just like @zekth said).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants