THRIFT-4171: Add setConnectTimeout to PHP TSocket#3531
Open
sveneld wants to merge 2 commits into
Open
Conversation
Client: php setSendTimeout() was used both as send timeout and as the timeout argument to fsockopen(), so callers had no way to set a short connect timeout without also shortening writes. Add setConnectTimeout(); if it is not called, open() falls back to setSendTimeout() for backwards compatibility. Generated-by: Claude Opus 4.7
Fires only when the caller actually called setSendTimeout() — i.e., the 'I tuned send timeout expecting it to also bound connect' usage that THRIFT-4171 is closing off. Default-constructed sockets (no setSendTimeout, no setConnectTimeout) keep the existing fallback silently so unrelated tests are not noisy. Generated-by: Claude Opus 4.7
Contributor
Author
|
Pushed dfc68b3 adding an |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves THRIFT-4171.
TSocket::setSendTimeout()was the only way to set the timeout passed tofsockopen()/pfsockopen(), so callers had to choose between a short connect timeout (good) and a short write timeout (often unwanted). Adds a separatesetConnectTimeout(int $timeout)method; if it is not called,open()falls back to the send timeout, preserving today's behaviour.Two unit tests cover both paths: the BC fallback and the override.
This addresses the same problem as the original PR #744, restructured around the typed properties introduced in recent PHP-library refactors.
THRIFT-NNNN: …[skip ci](n/a — code change)