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

Fix/warn file too big #8033

Merged
merged 7 commits into from Sep 29, 2020
Merged

Fix/warn file too big #8033

merged 7 commits into from Sep 29, 2020

Conversation

petersg83
Copy link
Contributor

@petersg83 petersg83 commented Sep 24, 2020

No modification where made for the providers aws and rackspace as they accept files up to 5 Go and the node buffer limit is 2 Go. So I couldn't test files bigger than 5 Go + the error would throw before the provider says it's too large.
It will be needed once we have change the buffers for streams.

@soupette @HichamELBSI I tag you because I updated the front part: the linter was not happy. Can you check everything's fine please?

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #8033 into master will increase coverage by 0.01%.
The diff coverage is 36.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8033      +/-   ##
==========================================
+ Coverage   32.71%   32.72%   +0.01%     
==========================================
  Files        1194     1196       +2     
  Lines       12969    13007      +38     
  Branches     1280     1285       +5     
==========================================
+ Hits         4243     4257      +14     
- Misses       7886     7905      +19     
- Partials      840      845       +5     
Flag Coverage Δ
#front 24.79% <0.00%> (-0.01%) ⬇️
#unit 53.77% <41.17%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...n/src/containers/NotificationProvider/selectors.js 0.00% <0.00%> (ø)
...cumentation/admin/src/containers/HomePage/index.js 0.00% <ø> (ø)
...ugin-upload/admin/src/components/EditForm/index.js 0.00% <0.00%> (ø)
.../containers/InputModalStepper/InputModalStepper.js 0.00% <ø> (ø)
.../src/containers/InputModalStepperProvider/index.js 0.00% <0.00%> (ø)
...-upload/admin/src/containers/ModalStepper/index.js 0.00% <ø> (ø)
packages/strapi-plugin-upload/services/Upload.js 15.75% <0.00%> (-0.45%) ⬇️
packages/strapi-plugin-upload/errors.js 42.10% <42.10%> (ø)
...strapi-plugin-upload/config/functions/bootstrap.js 64.10% <50.00%> (-3.55%) ⬇️
packages/strapi-provider-upload-local/lib/index.js 32.00% <50.00%> (+2.83%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd9abb4...f10aaf7. Read the comment docs.

Signed-off-by: Pierre Noël <petersg83@gmail.com>
Signed-off-by: Pierre Noël <petersg83@gmail.com>
Signed-off-by: Pierre Noël <petersg83@gmail.com>
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any changes to the aws and rackspace providers to handle this ? any reason ?

packages/strapi-plugin-upload/errors.js Outdated Show resolved Hide resolved
packages/strapi-plugin-upload/errors.js Outdated Show resolved Hide resolved
Signed-off-by: Pierre Noël <petersg83@gmail.com>
@@ -0,0 +1,8 @@
const errorTypes = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move that to the errors file and only expose the errors interface

Signed-off-by: soupette <cyril.lpz@gmail.com>
@soupette
Copy link
Contributor

@petersg83 seems like the linter is happy now.

Signed-off-by: Pierre Noël <petersg83@gmail.com>
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last comment :)

Signed-off-by: Pierre Noël <petersg83@gmail.com>
@alexandrebodin alexandrebodin added issue: bug Issue reporting a bug source: core:upload Source is core/upload package labels Sep 29, 2020
@alexandrebodin alexandrebodin added this to the 3.1.7 milestone Sep 29, 2020
@alexandrebodin alexandrebodin merged commit 567401f into master Sep 29, 2020
@alexandrebodin alexandrebodin deleted the fix/warnFileTooBig branch September 29, 2020 10:00
hdeadman pushed a commit to hdeadman/strapi that referenced this pull request Sep 29, 2020
* handle fileTooBig errors

Signed-off-by: Pierre Noël <petersg83@gmail.com>

* add entityTooLarge error in provider plugins

Signed-off-by: Pierre Noël <petersg83@gmail.com>

* fix linter

Signed-off-by: Pierre Noël <petersg83@gmail.com>

* refacto

Signed-off-by: Pierre Noël <petersg83@gmail.com>

* Add better error message for 413 errors in ML

Signed-off-by: soupette <cyril.lpz@gmail.com>

* refacto

Signed-off-by: Pierre Noël <petersg83@gmail.com>

* refacto

Signed-off-by: Pierre Noël <petersg83@gmail.com>

Co-authored-by: soupette <cyril.lpz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug source: core:upload Source is core/upload package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants