Skip to content

Stream proxied S3 and Azure artifacts - #8085

Draft
dralley wants to merge 1 commit into
pulp:mainfrom
dralley:streaming-file-fix
Draft

Stream proxied S3 and Azure artifacts#8085
dralley wants to merge 1 commit into
pulp:mainfrom
dralley:streaming-file-fix

Conversation

@dralley

@dralley dralley commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Bypass django-storages eager file buffering when the content app proxies S3 or Azure artifacts. Preserve the generic storage fallback and HTTP range behavior.

closes #7806

Assisted By: Codex (GPT-5) Terra 5.6

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

See: Pull Request Walkthrough

@dralley
dralley force-pushed the streaming-file-fix branch 5 times, most recently from 2f385d4 to 7d77c41 Compare September 11, 2026 02:43
Bypass django-storages eager file buffering when the content app
proxies S3 or Azure artifacts. Preserve the generic storage fallback
and HTTP range behavior.

closes pulp#7806

Assisted By: Codex (GPT-5) Terra 5.6
@dralley
dralley marked this pull request as ready for review September 11, 2026 03:22
@@ -0,0 +1,166 @@
"""Temporary private streaming adapters for object-storage response bodies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ideally none of these wrappers are necessary, and django-storages would allow us to do something like this natively: jschneier/django-storages#1568

But for now, we cannot, and the buffering implementation is genuinely unsuitable for sufficiently large files (context: a user has reported that they are storing files ranging up to 60 or even 300 gigabytes)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Obviously this is relatively invasive, so we need to be very careful with testing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, we could temporarily fork django-storages and add better APIs directly to the implementation. It is not in a great place maintenance-wise anyway currently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here is what the upstream PR looks like jschneier/django-storages#1569

@dralley
dralley requested a review from mdellweg September 11, 2026 03:35
@dralley
dralley marked this pull request as draft September 11, 2026 03:36
@dralley

dralley commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@mdellweg I'm marking this draft because it probably needs more extensive testing, however, I would still like feedback on the direction and approach. e.g. if you want to try the "vendor/fork django-storages" approach.

Both the issue itself and the upstream status of the library is a real issue we're going to need to deal with one way or the other.

@mdellweg

Copy link
Copy Markdown
Member

First, I don't know what the best approach is. Each approach has pro's and con's.
There might be a lightweight approach to vendoring, like we do with drf-spectacular. We monkeypatch the classes from there heavily including private interfaces. It comes with its own set of pain.

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.

Content app OOMs serving large S3 artifacts with REDIRECT_TO_OBJECT_STORAGE=False

2 participants