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

Add a Guzzle certificate client and add Guzzle 6 as a suggested dependency #25

Closed

Conversation

jeskew
Copy link
Contributor

@jeskew jeskew commented Aug 22, 2017

file_get_contents triggers errors and warnings rather than throwing exceptions, which can make recovering from a failure state tricky. This PR adds Guzzle as an optional dependency and provides a client that matches the interface expected by Aws\Sns\MessageValidator. Since Guzzle will also use cURL if it's available, this PR should resolve #23

/cc @kstich

@jeskew
Copy link
Contributor Author

jeskew commented Aug 22, 2017

The test of the optional Guzzle-based client cannot be run on PHP 5.4, so this PR also allows failures on that environment.

@kstich
Copy link
Contributor

kstich commented Aug 29, 2017

This feature is useful for consumers of the package but I don't think we should provide it in a mixed state of compatibility. This probably means taking a major version and updating the minimum PHP version to at least 5.5. We should be able to safely do this, as 5.4 is approaching two years past its EOL, but that should be a conscious decision.

@jeskew
Copy link
Contributor Author

jeskew commented Sep 9, 2017

@kstich I feel the same way. I originally wrote this mostly to serve as an example of how to replace file_get_contents, so perhaps it would work better in a documentation file (alongside the example provided in #23).

@github-actions
Copy link

github-actions bot commented Sep 8, 2020

We have noticed this issue has not recieved attention in 3 years. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

@yenfryherrerafeliz
Copy link
Contributor

Hi @jeskew, thanks for opening this PR. I will close this PR now due to its age, but please if you think this is still needed, please open a new PR over the latest changes in the code base.

Thanks!

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

Successfully merging this pull request may close these issues.

Basic curl implementation
3 participants