Update react/http to v1.0.0 - #504
Conversation
|
Seems this would be ok with a minor release as the changes are internal to ppm? |
|
Agreed, 👍 from my end This release might show some different performance characteristics mainly due to reactphp/http#371, so I think this warrants a |
|
What I actually wanted to suggest was a 2.0.5 as no APIs are changed for the typical use case as we rely on PSR for interfacing with the client application? |
|
I agree that no APIs have been changed, so I'll leave this decision up to you ultimately 👍 I would argue that in the SemVer sense ("breaking.feature.bug") this could be considered a feature rather than a bug fix (together with #503). I would consider "stable LTS" a feature on its own, but this might just be my personal preference. Either way works for me, let's just get this shipped |
|
I've been checking up on this project lately as we are experimenting with PHP-PM at our company and it will probably replace our PHP-FPM deployment in the coming months. I would just like to give an independent take on the 2.0.5 vs 2.1.0 debate as our use case sees a rare (but consistent) number of request bodies above 64K in size. This happens mostly when apps that were offline for a longer time send back some internal activity data. The number is way below 0.1% of all requests, but in this case this even exacerbates the issue, since we wouldn't pay that much attention to that and the problem would probably stay below our radar for some time. In our case, if the change would be in 2.0.5 and I wouldn't pay close attention to the changes in a minor release, this change would break those requests, so a 2.1.0 release would probably be a better choice. I agree that 64K is enough for the general use case, but often there are just enough exceptions that there might be issues when it comes to introducing such a change. Also, will it be possible to change these values via PHP-PM configuration so that we can adjust them for our use case? |
|
Thanks for your feedback- released at 2.1.0. @clue the last "unstable" component we have now is react/child-process ;) |
Awesome!
We're working on changing this as soon as time permits! 💪 You know how to help speed this up ;-) |
This minimal changeset updates react/http to the stable
v1.0.0released a few days ago: https://github.com/reactphp/http/releases/tag/v1.0.0The tests are broken as of #503 which applied an incomplete update of this component. I've updated the code to resolve some minor BC breaks, but this doesn't seem to be covered by the existing unit tests afaict. It looks like this should be covered by some of the integration tests that are defined in the Travis CI configuration, I'm fairly confident this should verify this piece of code.
Let me know if there's anything else that needs to be done to get this merged 👍