THRIFT-6023: Add HTTP transport support to PHP cross-tests#3515
Draft
sveneld wants to merge 12 commits into
Draft
THRIFT-6023: Add HTTP transport support to PHP cross-tests#3515sveneld wants to merge 12 commits into
sveneld wants to merge 12 commits into
Conversation
Client: php PHP was excluded from the cross-test HTTP matrix while other languages (Python, Go, Node, C++, Java, Lua, Dart, JS, D, Haskell) already declare "http" in their cross-test transports and verify interop. Server: TestServer.php detects --transport=http and pcntl_exec()s into PHP's built-in web server (php -S 127.0.0.1:$port TestServer.php). The same script re-enters under SAPI cli-server, reads the Thrift request via TPhpStream from php://input, dispatches it through the existing processor, and writes the response to php://output. Original -d flags from the launcher cmdline are preserved so the accel/sockets extensions stay loaded. Client: TestClient.php gains an --transport=http branch using TPsrHttpClient (THRIFT-6010) against http://127.0.0.1:\$port/, with no buffered/framed wrapper since TPsrHttpClient buffers internally. tests.json: adds "http" to PHP server and client transports and raises the client timeout from 6s to 10s to absorb HTTP overhead. composer.json: adds guzzlehttp/guzzle ^7.8 to require-dev so the PSR-18 auto-discovery in TPsrHttpClient resolves at cross-test time. Generated-by: Claude Opus 4.7
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 |
…tClient - TestServer.php cli-server branch peeks the Thrift message type; for ONEWAY it sends an empty HTTP 200 immediately (Content-Length: 0) and forks the handler so the client's oneway call returns fast — matches the THttpServer pattern from THRIFT-6021 and lets java-php HTTP work without known_failures entries. - Switch cli-server I/O to TMemoryBuffer; TPhpStream is no longer needed. - TestClient.php: replace 4-way if/elseif over \$MODE with two match expressions; drop the dead TSocket assignment and unused \$hosts/\$logger. - Remove php-cpp HTTP known_failures (unblocked by THRIFT-6021 / apache#3514). - Remove php-java HTTP known_failures (unblocked by fast oneway).
Contributor
Author
|
@kpumuk Thanks — addressed both points in 025863b:
While I was in there also collapsed the |
* lib/php/lib/Server/THttpServer.php (new): single-request Thrift HTTP server. Reads php://input, peeks the message type, and for ONEWAY sends an empty HTTP 200 immediately while running the handler in a forked child. Falls back to synchronous handling if pcntl is missing. Reusable outside the cross-test under any SAPI exposing php://input and php://output (php-fpm, mod_php, cli-server). * test/php/HttpRouter.php (new): minimal cli-server router invoked by `php -S`. Builds the processor + protocol factory and calls THttpServer::serve(). Replaces the SAPI-based branching in TestServer. * test/php/protocols.php (new): shared protocol-name → factory helper consumed by both TestServer.php (socket path) and HttpRouter.php (HTTP path). Uses error_log() rather than STDERR for cross-SAPI safety (STDERR is undefined under cli-server). * TestServer.php: drops the cli-server branch and the inline factory helper; the HTTP launcher now pcntl_exec's php -S with HttpRouter.php as the router script.
The class is test-infrastructure: the pcntl_fork + close-stdio pattern is cli-server specific and does not translate to php-fpm or mod_php (where fastcgi_finish_request() is the proper mechanism). Production PHP-Thrift over HTTP belongs in a framework controller, not this raw helper. Drop the lib/php/ entry and the Thrift\\Server namespace; the class lives next to its only caller (HttpRouter.php) as a global-scoped HttpServer, matching the convention of Handler.php in the same dir.
ThriftTest generated classes are not in Composer's PSR-4 map, so the classmap loader must be registered before Handler.php is required; otherwise `class Handler implements ThriftTestIf` fails at autoload and php -S falls back to a default response that confuses cross clients.
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.
Contributor
Author
|
Re-added the four |
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
2 participants
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.