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

Fix deprecated exceptions guzzle request option #1817

Merged
merged 2 commits into from
Oct 22, 2020

Conversation

deguif
Copy link
Collaborator

@deguif deguif commented Oct 20, 2020

This was deprecated in 6.0 version and a http_errors was introduced at this time. This won't work anymore with guzzle 7.x as exceptions option was removed.

@deguif
Copy link
Collaborator Author

deguif commented Oct 20, 2020

By the way I'm not sure which guzzle\guzzle versions are supported. It seems it's only 6.x currently.
@ruflin can you confirm?

@deguif
Copy link
Collaborator Author

deguif commented Oct 20, 2020

@ruflin should now be ready to merge.

@ruflin
Copy link
Owner

ruflin commented Oct 21, 2020

@deguif PR LGTM. For the guzzle question, TBH not sure. Why do you mean exactly by "supported"?

@deguif
Copy link
Collaborator Author

deguif commented Oct 21, 2020

By supported I mean the minimal version which works with ruflin/elastica when using guzzle transport shipped there.

@ruflin
Copy link
Owner

ruflin commented Oct 21, 2020

Good question, we haven't discuss this TBH. We should probably specify it?

@deguif deguif mentioned this pull request Oct 21, 2020
2 tasks
@deguif
Copy link
Collaborator Author

deguif commented Oct 21, 2020

A hard composer requirement would probably not be a good choice.
But adding a conflict could work, but can be sources of issues, as using ruflin/elastica doesn't mean you will use its guzzle transport.
Or at least check at runtime when using the guzzle transport, that would be the less intrusive solution.

@deguif
Copy link
Collaborator Author

deguif commented Oct 21, 2020

@ruflin I think this can be merged. The point about minimum version of guzzle/guzzle supported can be addressed in a future PR. Currently it seems it supports v6 and upper, but it needs some adaptations for v7 ;)

@deguif deguif force-pushed the fix-deprecated-guzzle-option branch from c2d1ca1 to d2a1e46 Compare October 22, 2020 09:28
@deguif deguif merged commit 682add9 into ruflin:master Oct 22, 2020
@deguif deguif deleted the fix-deprecated-guzzle-option branch October 22, 2020 09:37
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.

2 participants