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 streaming CSV export #52

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Fix streaming CSV export #52

merged 1 commit into from
Nov 22, 2024

Conversation

varyonic
Copy link
Owner

Streaming was not working either in development (disable_streaming off) nor on production Heroku. Adding a Last-Modified header made it work.

The problem would only manifest when using rack 2.2 or higher, and was caused by rack/rack#1416, where the Rack::ETag
middleware changed it's behaviour to buffer the response and calculate an ETag digest even though the Cache-Control: no-cache header is set. This breaks the CSV streaming responses so to avoid that we use the hack outlined in rack/rack#1619 and set a Last-Modified header which triggers the skip_caching? condition in the Rack::ETag middleware.

See activeadmin#6451 and alphagov/e-petitions@46413a7

Streaming was not working either in development (disable_streaming off)
nor on production Heroku. Adding a Last-Modified header made it work.

The problem would only manifest when using rack 2.2 or higher, and was
caused by rack/rack#1416, where the `Rack::ETag`
middleware changed it's behaviour to buffer the response and calculate
an ETag digest even though the `Cache-Control: no-cache` header is set.
This breaks the CSV streaming responses so to avoid that we use the hack
outlined in rack/rack#1619 and set a Last-Modified header which triggers
the `skip_caching?` condition in the `Rack::ETag` middleware.

See activeadmin#6451 and alphagov/e-petitions@46413a7
@varyonic varyonic merged commit ff03e38 into main Nov 22, 2024
8 checks passed
@varyonic varyonic deleted the fix/csv_streaming branch November 22, 2024 19:04
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.

1 participant