Adds guzzle http client library#38
Adds guzzle http client library#38kyrylo merged 3 commits intoairbrake:masterfrom danielpieper:guzzle
Conversation
|
Thanks for the PR! I assume this is a fix for #31. Looks like the build is failing, though. |
|
@kyrylo yes it's failing because the CircleCi uses an older version of PHP (5.4) but the guzzle library needs at least version 5.5. |
|
Sorry for delay, but I think that we need to add support for several HTTP-clients and then provide users ability to switch between raw PHP client and guzzle. Then we can consider making guzzle a default backend. |
| 'headers' => isset($http_response_header) ? $http_response_header : [], | ||
| 'data' => $respData, | ||
| ]; | ||
| $handler = (isset($this->opt['httpClient'])?$this->opt['httpClient']:null); |
There was a problem hiding this comment.
Can you please add spaces around ? and :?
|
Looks good. Can you please address my comments and squash commits so I can merge this? Thanks. |
|
|
||
| namespace Airbrake\Http; | ||
|
|
||
| use GuzzleHttp\Client; |
There was a problem hiding this comment.
Just wonder if this works when guzzle is not installed...
|
Thanks for your review, i fixed some issues but still tests and documentation/ readme are missing. i will add them soon |
|
Gentle ping @danielpieper. Would love to see this PR merged! :) |
|
@kyrylo I removed the (magic) client detection to stick to the live client's behavior. As the guzzle client library requires PHP >= 5.5 this test will be skipped on circle ci. |
Sends notifications using guzzle post requests
Removes (magic) client detection Adds factory test Disables guzzle test for php 5.4
| - Curl needs the curl php extension installed. See phpinfo(). | ||
| - The default client uses the php function "file_get_contents". Make sure | ||
| "allow_url_fopen" is set to "1" in your php.ini. | ||
| If not set the default client is used. |
There was a problem hiding this comment.
This is pretty vague. Could we mention what exactly this 'default' client is?
There was a problem hiding this comment.
this line describes the default client. Which information would you like to add?
There was a problem hiding this comment.
I was thinking about linking to the file, but seems like file_get_contents is well-known. Never mind :)
|
Thanks, @danielpieper! It looks fantastic. I just have a few questions/comments, but overall this looks good. |
3 participants
Sends notifications using guzzle post requests
Because of strict php.ini settings (allow_url_fopen=0) we cannot use file_get_contents to send out airbrake notifications. So i replaced it with the guzzle library.