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

Remove test for empty Content-Type #425

Merged
merged 1 commit into from
Jan 30, 2018
Merged

Conversation

nateprewitt
Copy link
Contributor

@nateprewitt nateprewitt commented Jan 28, 2018

Werkzeug 0.14.0 introduced new behaviour stripping falsey values from being included in EnvironHeaders (625362a). We were previously testing Content-Type header was empty which is now failing. From local tests with older versions of werkzeug, it looks like we weren't emitting the empty header in the /get or /anything responses.

I think we're probably OK removing these checks since they didn't seem to have had any production implications.

@sigmavirus24
Copy link
Contributor

Would it make sense to add 14.0.0 as the lowest possible version?

@nateprewitt
Copy link
Contributor Author

I'd say probably not since werkzeug isn't a direct dependency of httpbin. If we start requiring a specific version but leave Flask (which is what's currently installing it) unpinned, that may cause issues. Since we "don't see user facing impact"™ based on version, I think we should probably leave it up to the users' install of Flask to decide its dependency requirements.

@sigmavirus24
Copy link
Contributor

If we start requiring a specific version but leave Flask (which is what's currently installing it) unpinned

Having werkzeug >= 0.14.0 and Flask >= whatever wouldn't be objectionable to me.

I think we should probably leave it up to the users' install of Flask to decide its dependency requirements.

If folks are both using httpbin and Flask in a project, I'd be surprised and willing to rollback the lower version restrictions.

I suspect if there was a test encapsulating something here then we were testing it for some reason.

Werkzeug 14.0.0 introduced new behaviour stripping falsey
values from being included in EnvironHeaders.
@nateprewitt
Copy link
Contributor Author

From the details in #173 it looks like this was just testing an implementation detail, but like you said we can rollback if needed.

Pin for werkzeug added, couldn't find a modern version of Flask that it wasn't working with.

@sigmavirus24 sigmavirus24 merged commit 96c5e71 into master Jan 30, 2018
@sigmavirus24 sigmavirus24 deleted the remove_content_type_check branch January 30, 2018 15:15
virgiliojr94 pushed a commit to virgiliojr94/httpbin-chat2desk that referenced this pull request Jul 1, 2023
…ype_check

Remove test for empty Content-Type
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.

2 participants