Conversation
cds-plugin.js
Outdated
| cds.app.post(webhookBasePath, _validateCertificate.bind(this)) | ||
| } | ||
| cds.app.post(webhookBasePath, express.json()) | ||
| const limit = this.options.webhookSizeLimit ?? "1mb" |
There was a problem hiding this comment.
Why not use our official body parser limit?
https://pages.github.tools.sap/cap/docs/node.js/cds-server#maximum-request-body-size
There was a problem hiding this comment.
I think we should use the body parser limit and a dedicated option for webhooks only, like
body_parser_limit ?? webook_body_parser_limit
There was a problem hiding this comment.
I'd be in favor of webhook_body_parser_limit ?? body_parser_limit 😄
Since we would like to have an increased body size limit only for this event hub endpoint.
Co-authored-by: Andre Meyering <info@andremeyering.de>
David-Kunz
left a comment
David-Kunz
left a comment
There was a problem hiding this comment.
Tiny correction, otherwise okay!
Co-authored-by: Dr. David A. Kunz <david.kunz@sap.com>
| "requires": { | ||
| "messaging": { | ||
| "kind": "event-broker", | ||
| "webhookSizeLimit": "1mb" |
There was a problem hiding this comment.
also name it body_parser: { limit: ... } for consistency?
There was a problem hiding this comment.
I chose this name because it's more obvious what the parameter is used for in our case.
3 participants
Fixes #40