THRIFT-6023: Add HTTP transport support to PHP cross-tests#3515
Open
sveneld wants to merge 1 commit into
Open
Conversation
b0ecefc to
d26d7a6
Compare
5 tasks
Member
|
The last commit should hopefully be unnecessary with #3514. For Java, the issue is that it is the only cross-test that actually verifies the one-way semantics, as in that HTTP server must return the response immediately, and then run the handler afterwards (so that the client is as fast as possible). |
Contributor
Author
|
Thanks, i will update PR |
Contributor
Author
|
@kpumuk Thanks — addressed both points in 025863b:
While I was in there also collapsed the |
sveneld
added a commit
to sveneld/thrift
that referenced
this pull request
May 22, 2026
The cpp HTTP client cannot drain the empty oneway response that THttpServer-style servers (including our HttpServer.php) emit; per the maintainer note on apache#3515, THRIFT-6021 / apache#3514 fixes the cpp client side. Until that lands, these 4 combinations stay in known_failures to keep the matrix green; can be removed once apache#3514 is merged.
sveneld
added a commit
to sveneld/thrift
that referenced
this pull request
May 29, 2026
The cpp HTTP client cannot drain the empty oneway response that THttpServer-style servers (including our HttpServer.php) emit; per the maintainer note on apache#3515, THRIFT-6021 / apache#3514 fixes the cpp client side. Until that lands, these 4 combinations stay in known_failures to keep the matrix green; can be removed once apache#3514 is merged.
ffbaba0 to
0ef81ee
Compare
Client: php PHP was the only major-language entry stuck on buffered/framed sockets while Python, Go, Node, C++, Java, Lua, Dart, JS, D, and Haskell already declared http in their cross-test transports. Server (test/php/TestServer.php + HttpRouter.php + HttpServer.php): - On --transport=http, TestServer pcntl_exec()s into 'php -S' with HttpRouter.php as the router. The chameleon HttpRouter dispatches on PHP_SAPI: cli launches php -S with itself as the per-request handler; cli-server invokes HttpServer. - HttpServer reads php://input via TMemoryBuffer, peeks the message type, and for ONEWAY sends an empty HTTP 200 immediately while forking the handler so strict clients (Java) measure fast oneway latency. Sync path echoes the processor's response bytes. - protocols.php centralises the protocol-name -> TProtocolFactory dispatch, shared between the socket entry point and the cli-server router. Client (test/php/TestClient.php): - Adds an http branch that uses TPsrHttpClient (THRIFT-6010) against http://127.0.0.1:$port/. Drops the duplicate makeProtocol() helper in favour of thrift_test_protocol_factory()->getProtocol(), removes the unused $host argv parsing, and normalises the file to PSR-12. composer.json: adds guzzlehttp/guzzle ^7.8 to require-dev so PSR-18 discovery resolves a real client at cross-test time. test/tests.json: adds 'http' to PHP server and client transports. test/known_failures_Linux.json: lists php-cpp_*_http-ip (cpp THttpClient does not handle Connection: close between calls — tracked as THRIFT-6060) and nodejs-php_*_http-ip (Node http server mounts /test, not /, matching the existing cpp-/d-/go-nodejs known failures). Generated-by: Claude Opus 4.7
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
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.
Adds HTTP transport support to the PHP entry of the cross-test matrix. PHP was the only major-language entry stuck on
buffered/framedwhile Python, Go, Node, C++, Java, Lua, Dart, JS, D, and Haskell already declarehttpand verify HTTP interop.Server —
TestServer.phpdetects--transport=httpin CLI mode andpcntl_exec()s into PHP's built-in web server (php -S 127.0.0.1:$port TestServer.php). The same script re-enters under SAPIcli-server, reads the request viaTPhpStreamfromphp://input, dispatches it through the existing processor, and writes the response tophp://output. The original-dflags from the launcher cmdline (e.g.-dextension=thrift_protocol.so) are preserved by parsing/proc/self/cmdline, so the accel extension and sockets stay loaded under the spawned server too. The cross-runner'ssocket.connect_exreadiness probe works becausephp -STCP-binds the port immediately.Client —
TestClient.phpgains anif ($MODE == 'http')branch that usesTPsrHttpClient(added in THRIFT-6010) againsthttp://127.0.0.1:$port/.TPsrHttpClientbuffers internally, so noTBufferedTransport/TFramedTransportwrapping.tests.json — adds
"http"to PHP server and clienttransports; bumps clienttimeoutfrom 6s → 10s to absorb HTTP overhead.composer.json — adds
guzzlehttp/guzzle: ^7.8torequire-devso the PSR-18 auto-discovery inTPsrHttpClientresolves a concrete HTTP client at cross-test time.JIRA: THRIFT-6023. Draft until cross-test CI validates the new php↔php / php↔py3 / php↔go HTTP matrix cells.
[skip ci]anywhere in the commit message to free up build resources.