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

Redirect client to download media directly from Amazon S3 / Azure Storage #2713

Merged
merged 21 commits into from
Oct 14, 2024

Conversation

kelvin-muchiri
Copy link
Contributor

@kelvin-muchiri kelvin-muchiri commented Sep 30, 2024

Changes / Features implemented

Redirect client to download media directly from Amazon S3 / Azure Storage. Before, the server was responsible for reading the file contents and streaming the data back to the client. This implementation was prone to timeouts if the file is large and required the server to adjust UWSGI and Nginx timeouts to large values.

Deployments that choose to use local storage are not affected by the change.

Steps taken to verify this change does what is intended

  • QA

Side effects of implementing this change

Prevents large exports from timing out

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

Closes #

@kelvin-muchiri kelvin-muchiri marked this pull request as ready for review October 3, 2024 08:34
@kelvin-muchiri kelvin-muchiri changed the title Download exports directly from S3 Download media directly from S3 Oct 3, 2024
@kelvin-muchiri kelvin-muchiri changed the title Download media directly from S3 Redirect client to download media directly from Amazon S3 / Azure Storage Oct 3, 2024
Comment on lines 1743 to 1746
# # Anonymous user cannot view private EntityList
# request = self.factory.get("/")
# response = self.view(request, pk=self.entity_list.pk)
# self.assertEqual(response.status_code, 404)
Copy link
Member

@FrankApiyo FrankApiyo Oct 8, 2024

Choose a reason for hiding this comment

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

Does this mean that anonymous users can now view private EntityLists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FrankApiyo Nice catch, I don't think this was intentional

request = self.factory.get("/export")
force_authenticate(request, user=self.user)
response = self.view(request, pk=export.pk)
self.assertEqual(response.status_code, 302, response.url)
Copy link
Member

Choose a reason for hiding this comment

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

What does the 3rd argument to assertEqual do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FrankApiyo I copied the assertion from a previous implementation. I will remove it since the 3rd argument should be the message displayed if the assertion fails.

Comment on lines -752 to +878
return f"attachment; filename={name}.{extension}"
# The filename is enclosed in quotes because it ensures that special characters,
# spaces, or punctuation in the filename are correctly interpreted by browsers
# and clients. This is particularly important for filenames that may contain
# spaces or non-ASCII characters.
Copy link
Member

Choose a reason for hiding this comment

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

👍🏿 👍🏿

@kelvin-muchiri kelvin-muchiri enabled auto-merge (squash) October 14, 2024 09:21
@kelvin-muchiri kelvin-muchiri merged commit d17dd40 into main Oct 14, 2024
10 checks passed
@kelvin-muchiri kelvin-muchiri deleted the exports-download-url branch October 14, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants