Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

[Http] Add body support for DELETE/OPTIONS request #6492

Closed
wants to merge 2 commits into from
Closed

[Http] Add body support for DELETE/OPTIONS request #6492

wants to merge 2 commits into from

Conversation

egeloen
Copy link
Contributor

@egeloen egeloen commented Jul 28, 2014

Hey!

This PR adds support for request body with other methods such as DELETE/OPTIONS with the curl client. Currently, it is only supported with the socket client and so, this PR rationalized the behavior between all clients.

@Ocramius
Copy link
Member

Seems related to #6318, which will land only in 2.4.

Could you eventually add a test, especially for OPTIONS (which is new here)?

@Ocramius Ocramius added this to the 2.4.0 milestone Jul 28, 2014
@egeloen
Copy link
Contributor Author

egeloen commented Jul 28, 2014

@Ocramius Should I add some tests which will be specific to curl or shared by all adapters?

@Ocramius
Copy link
Member

@egeloen whatever makes more sense for you

@egeloen
Copy link
Contributor Author

egeloen commented Jul 29, 2014

@Ocramius Done. With these tests, I have realized an other method needs to be updated accordingly.

Additionally, by running tests which requires a web server, I have noticed some tests are failing due to this line and this line (the max age is equal to -2400).

@Ocramius
Copy link
Member

@weierophinney have we ever tried enabling the HTTP integration tests on travis?

@egeloen
Copy link
Contributor Author

egeloen commented Jul 30, 2014

I can enable it on travis but which web server should I use? nginx, apache, an other?

@Ocramius
Copy link
Member

We can use the internal PHP Server

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 30 July 2014 16:38, Eric GELOEN notifications@github.com wrote:

I can enable it on travis but which web server should I use? nginx,
apache, an other?


Reply to this email directly or view it on GitHub
#6492 (comment).

@egeloen
Copy link
Contributor Author

egeloen commented Aug 3, 2014

I just try to use the PHP built in web server and there is lot of tests which fails (more than with a real web server). So, I'm not sure it's a good idea to rely on it.

@Ocramius
Copy link
Member

Ocramius commented Aug 4, 2014

I'm not sure if travis comes with any pre-installed web server, because I don't think we should install one during builds either.

Yes, the internal PHP server is indeed very simplified: I was hoping for it to support our necessities :\

@egeloen
Copy link
Contributor Author

egeloen commented Aug 4, 2014

Unfortunately, travis does not come with a web server as pointed here. We need to install/configure it during the build. So, if you think it's a no go to install it, I can't do anything :s

@Ocramius
Copy link
Member

Ocramius commented Aug 4, 2014

Re-assigning to @weierophinney - I'll let this decision to him. The question here is build times, mainly.

@egeloen
Copy link
Contributor Author

egeloen commented Oct 25, 2014

@weierophinney Can you please give me your opinion?

@Ocramius Ocramius modified the milestones: 2.3.4, 2.4.0 Nov 19, 2014
Ocramius added a commit that referenced this pull request Nov 19, 2014
Ocramius added a commit that referenced this pull request Nov 19, 2014
@Ocramius Ocramius closed this in 78a6c25 Nov 19, 2014
@Ocramius
Copy link
Member

@egeloen I've manually merged this PR into master and develop, thanks!

master: 78a6c25
develop: 7a1c743

@egeloen
Copy link
Contributor Author

egeloen commented Nov 19, 2014

@Ocramius Thank you too!

@egeloen egeloen deleted the http-curl-body branch November 19, 2014 20:14
Ocramius added a commit that referenced this pull request Nov 20, 2014
Ocramius added a commit that referenced this pull request Nov 20, 2014
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
…, removed duplicate/redundant `elseif` statements
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants