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

[Task]: Reviewer tools: Support attaching a zip file to a review #14999

Closed
1 task
wagnerand opened this issue Sep 3, 2024 · 9 comments · Fixed by mozilla/addons-server#22670
Closed
1 task
Assignees
Labels
qa:verified_fix repository:addons-server Issue relating to addons-server
Milestone

Comments

@wagnerand
Copy link
Member

wagnerand commented Sep 3, 2024

Description

To reduce reviewer workload by doing the same task of building an artifact for a developer to check repeatedly and to eliminate the dependency on a third-party service, the reviewer tools should support adding a zip attachment to at least rejections and reviewer replies. If it's not too much additional effort, ideally it should be supported for all actions.

Acceptance Criteria

Milestones/checkpoints

Checks

  • If the issue is ready to work on, I have removed the "needs:info" label.
  • If I have identified that the work is specific to a repository, I have removed "repository:addons-server" or "repository:addons-frontend"

┆Issue is synchronized with this Jira Task

@wagnerand wagnerand added needs:info repository:addons-server Issue relating to addons-server labels Sep 3, 2024
@wagnerand wagnerand changed the title [Task]: Reviewer tools. Support attaching a zip file to a review [Task]: Reviewer tools: Support attaching a zip file to a review Sep 3, 2024
@chrstinalin chrstinalin self-assigned this Sep 5, 2024
@diox
Copy link
Member

diox commented Sep 10, 2024

Let's start with a 100 MB file limit to begin with and see how it goes.

@diox
Copy link
Member

diox commented Sep 10, 2024

Note: zip files should also go through SafeZip like all our zip files.

@diox diox removed the needs:info label Sep 10, 2024
@wagnerand
Copy link
Member Author

Let's do 200MB. Several dozen versions submitted just this year have XPI files larger than 100MB and at least one of them is Notable.

@diox
Copy link
Member

diox commented Sep 25, 2024

(We did 210 MB / 200 MiB)

Depends on #14997

@ioanarusiczki
Copy link

ioanarusiczki commented Sep 26, 2024

I've errors on dev with 1 + MB zip files, I tried from win and macOS. Same with a larger txt file.

2 85 zip

@eviljeff
Copy link
Member

@ioanarusiczki large files should be working now (on dev and stage)

@ioanarusiczki
Copy link

I tried on dev and stage to make uploads.

Tested on:
https://reviewers.addons.allizom.org/en-US/reviewers/review-listed/1031481
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/633303

What passed: zip files attached to various reviewer actions (can be seen on the links above) of 3, 9, 13, 20, 26, 37 MB
Also a 9MB txt file.

I've noticed that somewhere from 40MB + , I've the 502 server error so it failed with:
44, 48, 50, 116 mb zip files

502server error

I tested this on both dev and stage servers.

@ioanarusiczki
Copy link

Larger uploads on dev went through for: 50, 120, 150 up to 180+ MB

https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-unlisted/633177#review-actions

180 plus on stage too https://reviewers.addons.allizom.org/en-US/reviewers/review-listed/1031488#review-actions

At 190 + I'm getting this error

at 200

Anything above 200 MB throws the error

202

@ioanarusiczki
Copy link

There's a followup filed #15093

I'm marking this verified since large uploads are now working as mentioned above on dev and stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa:verified_fix repository:addons-server Issue relating to addons-server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants