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

request.POST now supports any requests with the appropriate Content-Type (#351) #352

Merged
merged 2 commits into from Apr 5, 2018

Conversation

Projects
None yet
2 participants
@Cykooz
Copy link
Contributor

Cykooz commented Mar 8, 2018

Issue #351

@Cykooz Cykooz changed the title ``request.POST`` now supports DELETE requests with the appropriate Content-Type (#351) request.POST now supports DELETE requests with the appropriate Content-Type (#351) Mar 8, 2018

@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented Mar 8, 2018

Instead of limiting it to just certain request methods, it may be better to just look for content-type if it is not a POST, and if it is form-encoded then we can continue.

This way the WebDav request methods also just work.

"""
env = self.environ
if self.method not in ('POST', 'PUT', 'PATCH'):
if self.method not in ('POST', 'PUT', 'PATCH', 'DELETE'):

This comment has been minimized.

@bertjwregeer

bertjwregeer Mar 8, 2018

Member

I would argue for removal of this entirely, so that it just works on all request methods, Webdav has a bunch of other request methods for example and it doesn't make sense if we disallow .POST on those.

As long as we keep the line on 766 we should be good to go.

This comment has been minimized.

@Cykooz

Cykooz Mar 8, 2018

Contributor

It is a cool idea. I will remove this condition.

@Cykooz Cykooz changed the title request.POST now supports DELETE requests with the appropriate Content-Type (#351) request.POST now supports any requests with the appropriate Content-Type (#351) Mar 8, 2018

@Cykooz

This comment has been minimized.

Copy link
Contributor

Cykooz commented Mar 18, 2018

Ping-ping.

What about merging this PR?

@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented Mar 20, 2018

I've been busy with work and the flu. I'm also traveling at the moment. I'll get this merged before I push the release for 1.8.

@bertjwregeer bertjwregeer added this to the 1.8.0 milestone Mar 20, 2018

@bertjwregeer bertjwregeer merged commit 1f91bcc into Pylons:master Apr 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bertjwregeer

This comment has been minimized.

Copy link
Member

bertjwregeer commented Apr 5, 2018

Thank you!

bertjwregeer added a commit that referenced this pull request Apr 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment