Skip to content
This repository has been archived by the owner on Jun 18, 2020. It is now read-only.

Fix: change node express max request body size to 50MB #8

Closed
wants to merge 1 commit into from
Closed

Conversation

haozibi
Copy link

@haozibi haozibi commented Sep 11, 2019

The node Express default payload is very small, only about 100kb. When the request payload is large, it will report "Payload Too Large". so the payload size is limited to 50MB.

I feel that HTTP mode also needs to provide several configuration items according to the framework.

@derek
Copy link

derek bot commented Sep 11, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@derek derek bot added the no-dco label Sep 11, 2019
Signed-off-by: haozibi <haozibi@foxmail.com>
@derek derek bot removed the no-dco label Sep 11, 2019
@rimiti
Copy link

rimiti commented Sep 11, 2019

It's not a good practice to accept big payload. You should avoid that.
Anyway, I not sure about merging this PR. It depends on your use case.

@haozibi
Copy link
Author

haozibi commented Sep 12, 2019

It's not a good practice to accept big payload. You should avoid that.
Anyway, I not sure about merging this PR. It depends on your use case.

Hello, @rimiti express default bodyParser size is too small (default 100kb), set 50MB just to avoid this problem, you can add configuration items to configure later

Many people have encountered this problem.

@alexellis
Copy link
Member

Hi both @haozibi @rimiti

Every PR to OpenFaaS first needs and issue to propose the change.

This is well documented within the contributing guide.

Please could you raise an issue and outline the problem, the proposed solution and any pros/cons that come with it?

In particular I do not believe that hard-coding a 50MB limit comes without cons. As a rule we prefer to make things configurable when the default doesn't suit every user. FYI configuration is usually done via code or via environment variable.

Alex

@haozibi
Copy link
Author

haozibi commented Sep 12, 2019

Hi both @haozibi @rimiti

Every PR to OpenFaaS first needs and issue to propose the change.

This is well documented within the contributing guide.

Please could you raise an issue and outline the problem, the proposed solution and any pros/cons that come with it?

In particular I do not believe that hard-coding a 50MB limit comes without cons. As a rule we prefer to make things configurable when the default doesn't suit every user. FYI configuration is usually done via code or via environment variable.

Alex

Ok, I know it

@haozibi haozibi closed this Sep 12, 2019
@alexellis
Copy link
Member

I would like to see the max body size become configurable.

Let's talk on the new issue that you are going to raise now?

Alex

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

Successfully merging this pull request may close these issues.

None yet

3 participants