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.

Sign up for GitHub

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

Jump to bottom

[Migrated] Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary #908

Closed
jneves opened this issue Feb 20, 2021 · 3 comments · Fixed by #1155
Closed

[Migrated] Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary #908

jneves opened this issue Feb 20, 2021 · 3 comments · Fixed by #1155
Assignees
@monkut
Labels
has-pr next-release-candidate

Comments

@jneves
Copy link
Contributor

jneves commented Feb 20, 2021

Originally from: Miserlou/Zappa#2170 by monkut

Description

Alternative solution to Miserlou/Zappa#2080 intended to allow BINARY and text by making use of Content-Encoding header in the response.

When using whitenoise for caching, which provides compression, binary types may include mimetypes, "text/", "application/json":

  • response.mimetype.startswith("text/")
  • response.mimetype == "application/json"

Assuming that Content-Encoding will be set (as whitenoise apparently does) this allows compression to be applied to assets which have mimetypes "text/" and "application/json", while allowing for uncompressed "text/" and "application/json" still to be served.

About Content-Encoding:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding

Not sure if this works for all use cases, but it currently appears to work for my workcase where whitenoise is performing compression of text/json files intended for caching, while still allowing text for html.

GitHub Issues

Miserlou/Zappa#2080
@Quidge
Copy link

Quidge commented Apr 8, 2021 •
edited
Loading

Our team also ran into this issue. We forked the Zappa repo and merged this older PR: bfcb649 (#2141).

@jneves Is there anything you would like to see that could help this older PR code to get over the line and into main? AFAICT it didn't pass original CI because it decreased coverage by 0.03%.

I'd be happy to add some tests for this scenario and increase coverage.

Quidge added a commit to tackle-io/Zappa that referenced this issue Apr 28, 2021
@Quidge
Write tests confirming Binary Support response behavior
366cc3c 
No functional change to the Zappa codebase is introduced by this commit.

This area of the application was untested. Tests introduced to ensure new
behavior discussed in zappa#908 does not cause
a regression.
@Quidge Quidge mentioned this issue Apr 28, 2021
Closed
@cdruck
Copy link

cdruck commented Jun 1, 2021

This also causes failure when it is useful to compress JSON API responses ...

Quidge added a commit to tackle-io/Zappa that referenced this issue Jun 2, 2021
@Quidge
Write tests confirming Binary Support response behavior
5bfa5dd 
No functional change to the Zappa codebase is introduced by this commit.

This area of the application was untested. Tests introduced to ensure new
behavior discussed in zappa#908 does not cause
a regression.
Quidge added a commit to tackle-io/Zappa that referenced this issue Nov 25, 2021
@Quidge
Write tests confirming Binary Support response behavior
b5942d9 
No functional change to the Zappa codebase is introduced by this commit.

This area of the application was untested. Tests introduced to ensure new
behavior discussed in zappa#908 does not cause
a regression.
Quidge added a commit to tackle-io/Zappa that referenced this issue Nov 29, 2021
@Quidge
Write tests confirming Binary Support response behavior
0125954 
No functional change to the Zappa codebase is introduced by this commit.

This area of the application was untested. Tests introduced to ensure new
behavior discussed in zappa#908 does not cause
a regression.
@monkut monkut mentioned this issue Dec 8, 2021
Closed
This was referenced Jul 16, 2022
Closed
Closed
Closed
Closed
Closed
@monkut monkut self-assigned this Jul 28, 2022
@monkut monkut added the has-pr label Jul 28, 2022
@monkut monkut mentioned this issue Jul 28, 2022
Merged
@monkut monkut linked a pull request Jul 28, 2022 that will close this issue
(#908) Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary #1155
Merged
@monkut monkut mentioned this issue Aug 16, 2022
Closed
monkut added a commit that referenced this issue Nov 11, 2022
@monkut
(#908) Update BINARY_SUPPORT to use Content-Encoding to identify if d…
bad24dd 
…ata is binary (#1155)

* 🔧 migrate #971 to lastest master

* 🎨 run black/isort

* ♻️ refactor to allow for other binary ignore types based on mimetype. (currently openapi schema can't be passed as text.

* 🎨 run black/fix flake8

* 🔧 add EXCEPTION_HANDLER setting

* 🐛 fix zappa_returndict["body"] assignment

* 📝 add temp debug info

* 🔥 delete unnecessary print statements

* ♻️ Update comments and minor refactor for clarity

* ♻️ refactor for ease of testing and clarity

* 🎨 fix flake8

* ✨ add `additional_text_mimetypes` setting
✅ add testcases for additional_text_mimetypes handling

* 🔧 Expand default text mimetypes mentioned in #1023
♻️ define "DEFAULT_TEXT_MIMETYPES" and move to utilities.py

* 🎨 run black/isort

* 🎨 run black/isort

* 🎨 remove unnecesasry comment (black now reformats code)
🎨 change commented lines to docstring for test app
@monkut monkut mentioned this issue Nov 12, 2022
Merged
@monkut
Copy link
Collaborator

monkut commented Dec 1, 2022

released in 0.56.0, closing.

Ian288 pushed a commit to tackle-io/Zappa that referenced this issue Jul 11, 2023
@monkut @Ian288
(zappa#908) Update BINARY_SUPPORT to use Content-Encoding to identify…
b9c5980 
… if data is binary (zappa#1155)

* 🔧 migrate zappa#971 to lastest master

* 🎨 run black/isort

* ♻️ refactor to allow for other binary ignore types based on mimetype. (currently openapi schema can't be passed as text.

* 🎨 run black/fix flake8

* 🔧 add EXCEPTION_HANDLER setting

* 🐛 fix zappa_returndict["body"] assignment

* 📝 add temp debug info

* 🔥 delete unnecessary print statements

* ♻️ Update comments and minor refactor for clarity

* ♻️ refactor for ease of testing and clarity

* 🎨 fix flake8

* ✨ add `additional_text_mimetypes` setting
✅ add testcases for additional_text_mimetypes handling

* 🔧 Expand default text mimetypes mentioned in zappa#1023
♻️ define "DEFAULT_TEXT_MIMETYPES" and move to utilities.py

* 🎨 run black/isort

* 🎨 run black/isort

* 🎨 remove unnecesasry comment (black now reformats code)
🎨 change commented lines to docstring for test app
BarNehemia added a commit to Lightricks/Zappa that referenced this issue Aug 10, 2023
@BarNehemia
Merge commit '0b1eab14ca39c3a3bfb4e915347e07495171dcba' into upgrade-…
93295d2 
…zappa-0.57.0

* commit '0b1eab14ca39c3a3bfb4e915347e07495171dcba': (27 commits)
  updating workflow actions to remove deprecation warnings (zappa#1243)
  📝 CHANGELOG.md update for 0.57.0 (zappa#1246)
  fixing compatibility with Django 4.2 (zappa#1237)
  Update Readme with patreon and donors (zappa#1234)
  adding support for Python 3.10 (zappa#1231)
  Nose to pytest (zappa#1239)
  lint: updating code style with `make black` (zappa#1238)
  Alternative way to check if running in Docker (zappa#1204)
  📝 CHANGELOG.md update for 0.56.0 release (zappa#1187)
  Improve `get_best_match_zone()` to match by most matched components instead of by length of domain (zappa#1193)
  Bypass python version runtime check when code run in docker (zappa#1180)
  Be able to pass in a batch window when using batch size (zappa#1118)
  Correction to README. (zappa#1177)
  (zappa#908) Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary (zappa#1155)
  Remove special check for Django<1.7, fix for zappa#1158  (zappa#1159)
  Resolve (zappa#410) Logs are missing query strings (zappa#1165)
  Handle spaces in x-forwared-for/remove six (zappa#1127) (zappa#1163)
  add feature (zappa#704) Check if args/kwargs are JSON Serializable while running locally (zappa#1154)
  docs: Add documentation for s3 event object key_filters (zappa#1169)
  Add feature pyenv virtualenv detecting .python-version file (zappa#1153)
  ...

# Conflicts:
#	zappa/cli.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@monkut monkut

Labels
has-pr next-release-candidate
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(#908) Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary zappa/Zappa
4 participants
@jneves @monkut @Quidge @cdruck