Skip to content

THRIFT-6498: Fix the PHP tutorial server and its runserver.py - #4158

Open
slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-6498
Open

slachiewicz wants to merge 1 commit into
apache:masterfrom
slachiewicz:THRIFT-6498

Conversation

@slachiewicz

Copy link
Copy Markdown
Member

Jira: THRIFT-6498

The PHP tutorial server no longer loaded: since THRIFT-6004 the generated CalculatorIf and SharedServiceIf declare native types, so the untyped handler in PhpServer.php is a fatal error. The handler now uses the generated signatures. Its header() call also lacked the colon, which php -S sent as a malformed header line, so THttpClient could not read the replies.

runserver.py was Python 2 only (BaseHTTPServer, and the CGI handler is removed in Python 3.15). It now starts php -S on port 8080, and the server's header comment says so. Makefile.am depended on gen-php/tutorial/Calculator.php, which is no longer generated; it now uses CalculatorIf.php.

Verified: on master php PhpServer.php fails with "Declaration of CalculatorHandler::ping() must be compatible with CalculatorIf::ping(): void"; on this branch, after thrift --gen php:server -r ../tutorial.thrift, python3 runserver.py and php PhpClient.php --http print ping, 1+1=2, the divide-by-zero InvalidOperation, 15-10=5 and the getStruct log.

  • Did you create an Apache Jira ticket?
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-6498: describe my issue"?
  • Did you squash your changes to a single commit?
  • Did you do your best to avoid breaking changes? Tutorial files only.

Client: php

Since THRIFT-6004 the generated CalculatorIf and SharedServiceIf carry native
types, so the untyped handler in PhpServer.php was a fatal error. The handler
now uses the generated signatures. The header() call lacked the colon, which
php -S sent as a malformed header line, so THttpClient failed to read replies.

runserver.py was Python 2 only (and CGIHTTPRequestHandler is deprecated); it
now starts php -S on port 8080, which needs no shebang line in PhpServer.php.
Makefile.am depended on gen-php/tutorial/Calculator.php, which is no longer
generated; it now uses CalculatorIf.php.

Tested: generated the tutorial code, ran runserver.py and PhpClient.php --http.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@mergeable mergeable Bot added php build and general CI cmake, automake and build system changes labels Nov 8, 2026
@slachiewicz
slachiewicz marked this pull request as ready for review November 8, 2026 20:16
@slachiewicz
slachiewicz requested a review from sveneld as a code owner November 8, 2026 20:16
Copilot AI balanced review requested due to automatic review settings November 8, 2026 20:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build and general CI cmake, automake and build system changes php

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants