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 Pastebin Re-implementation #14997

Closed
3 tasks done
chrstinalin opened this issue Sep 3, 2024 · 10 comments · Fixed by mozilla/addons-server#22640
Closed
3 tasks done

[Task]: Reviewer Tools Pastebin Re-implementation #14997

chrstinalin opened this issue Sep 3, 2024 · 10 comments · Fixed by mozilla/addons-server#22640
Assignees
Labels
qa:verified_fix repository:addons-server Issue relating to addons-server
Milestone

Comments

@chrstinalin
Copy link

chrstinalin commented Sep 3, 2024

Description

Reviewers need a way to communicate build logs with developers and rely on Pastebin to do so. With its decommission, this task is a part of the re-implementation of pastebin.mozilla.org on the Reviewer Tools side.

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

@chrstinalin chrstinalin added the repository:addons-server Issue relating to addons-server label Sep 3, 2024
@diox
Copy link
Member

diox commented Sep 4, 2024

Note: for storage, please use id_to_path(<id>, breadth=2) in the upload_to parameter of the FileField, and a dedicated folder under shared_storage (your storage parameter should be set to SafeStorage(root_setting='MEDIA_ROOT'))

Then we'll need to communicate that storage path to SRE to adjust the nginx configuration in dev/stage/prod.

@ioanarusiczki
Copy link

I don't see this on -dev yet but maybe because of what @diox said above.

@diox
Copy link
Member

diox commented Sep 18, 2024

Work is technically done but won't be enabled yet, it's behind a waffle switch, so moving to the next milestone.

@diox
Copy link
Member

diox commented Sep 25, 2024

@ioanarusiczki : Waffle switch to enable this feature is enable-activity-log-attachments. It should work on dev now that Wei has done https://mozilla-hub.atlassian.net/browse/SVCSE-2218

@ioanarusiczki
Copy link

I started looking over, I've seen there is a switch. So far:

  • all reviewer actions have this attachment possibility including unreject or flag for human review
  • the download attachment throw a 404 when clicked. I tried with https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/633005.
  • I could upload small zip files, when I try something a bit larger (actually I used zipped extensions of 2 MB or Ublock origin , which is 3MB+), I get a 413 error

413

  • the Paste from Clipboard worked from win10, the download throws an error (same as for the zip files attached)
  • I did not try all the possible reviewer actions to check the emails, but that would be [Task]: Pastebin - Include link to any attachment in any action #15018. So far, I did notice that reviewer's reply email has this but I did not see a reference to it for approve or reject (will re-check anyway).

@chrstinalin
Copy link
Author

chrstinalin commented Sep 25, 2024

For sanity, double-checked master branch locally --

@diox
Copy link
Member

diox commented Sep 25, 2024

@chrstinalin: The 404 is the addons-frontend one, so it seems we're missing the nginx config to route these kind of URLs to addons-server. Did we file the ticket for SRE to do that ?

@chrstinalin
Copy link
Author

@diox https://mozilla-hub.atlassian.net/browse/SVCSE-2218 was the only one I was aware was needed

@diox
Copy link
Member

diox commented Sep 25, 2024

There are 2:

  • We need the storage path (on the filesystem) for the attachments to be set as internal in nginx, allowing HttpResponseXSendFile (which does a X-Accel-Redirect) to work. That's https://mozilla-hub.atlassian.net/browse/SVCSE-2218
  • We need the URL for the attachements to be served by addons-server, as by default nginx will route things to addons-frontend. That's missing, since /activity/... is an entirely new kind of URL we've added for this issue. That was discussed at some point but maybe not written so we forgot to file that issue, my bad.

Could you file it? Example from a previous time we needed to do that some time ago: https://mozilla-hub.atlassian.net/browse/SVCSE-1563

@ioanarusiczki
Copy link

ioanarusiczki commented Oct 2, 2024

I'm going to mark this verified, I enabled the switch on stage and for any reviewer action I could:

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.

3 participants