Skip to content

Added cURL backup method to ApiClient - #48

Merged
jpignata merged 1 commit into
qltysh-archive:masterfrom
aensley:master
Apr 18, 2016
Merged

jpignata merged 1 commit into
qltysh-archive:masterfrom
aensley:master

Conversation

@aensley

@aensley aensley commented Apr 18, 2016 •

Copy link
Copy Markdown
Contributor

Attempting to solve the following error in Travis-CI builds with allow_url_fopen disabled:

Unexpected response: 500 fopen(https://codeclimate.com/test_reports): failed to open stream: no suitable wrapper could be found

Example project and build log:
https://github.com/aensley/YahooStockQuotes
https://travis-ci.org/aensley/YahooStockQuotes/jobs/123515259#L314

@jpignata

Copy link
Copy Markdown
Contributor

@aensley A couple of random (probably dumb – sorry 😄 ) questions:

  1. Is it possible for us to toggle this runtime configuration in our test reporter for the duration of the send if it's disabled?
  2. If not, would it perhaps be better if we always used libcurl?

@aensley

aensley commented Apr 18, 2016 •

Copy link
Copy Markdown
Contributor Author

@jpignata Those questions aren't dumb at all.

  1. I'm not sure in this specific case. Many (most?) variables can be changed with ini_set(), but not all.
  2. It might be better to default to cURL and fallback to PHP Streams. If the user does not have cURL installed, obviously, they'll need the fallback. I've never seen anyone complain about not having cURL but lots have complained about not having code that uses it :-p.

Since I was adding an alternative, I kept the current/previous method as the default. I can change this if you think it would be better.

@aensley

aensley commented Apr 18, 2016

Copy link
Copy Markdown
Contributor Author

I could see it not being possible to change this at run-time. It's a security-related option, so I doubt it could be changed at runtime. Safe Mode also disables allow_url_fopen, and I'm certain it can't be changed in that situation.

@jpignata

Copy link
Copy Markdown
Contributor

@aensley Makes sense. Thanks!

@jpignata
jpignata merged commit a8226e7 into qltysh-archive:master Apr 18, 2016
@aensley

aensley commented Apr 18, 2016

Copy link
Copy Markdown
Contributor Author

Awesome. Thanks!

@aensley

aensley commented Apr 18, 2016

Copy link
Copy Markdown
Contributor Author

@aensley

aensley commented Apr 18, 2016 •

Copy link
Copy Markdown
Contributor Author

You might want to remove this line to reduce output:

curl_setopt($curl, CURLOPT_VERBOSE, true);

I forgot to take that out. Sorry.

https://curl.haxx.se/libcurl/c/CURLOPT_VERBOSE.html

aensley added a commit to aensley/php-test-reporter that referenced this pull request Apr 19, 2016
@aensley aensley mentioned this pull request Apr 19, 2016
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