PHP SDK Relay Server #62
Conversation
typotter
requested review from
leoromanovsky,
aarsilv and
greghuels
and removed request for
leoromanovsky,
aarsilv and
greghuels
| ### With Docker | ||
| #### Build Docker Image | ||
| ```shell | ||
| docker build -t Eppo-exp/php-sdk-relay . |
| $subjectAttributes = $payload['subjectAttributes']; | ||
|
|
||
| try { | ||
| if (isset(self::$methods[$variationType])) { |
There was a problem hiding this comment.
consider making this a guard clause to spatially locate the error being thrown with why
if (!isset(self::$methods[$variationType])) {
throw new Exception("Invalid variation type $variationType");
}
...| */ | ||
| public function getAssignment(array $payload): array | ||
| { | ||
| $variationType = $payload['variationType']; |
There was a problem hiding this comment.
It may be too late in the game to change this, but "variation type" is not a good descriptor for this. Variation Type already means the type of value variations hold (e.g., booleans, strings, integer, numeric JSON) and now were overloading it to also mean the type of assignment we are requesting (note: bandits return string variation types... for now!).
I think a better name for this would be assignment type.
Sorry I didn't think of this until now
There was a problem hiding this comment.
👍 Let's plumb it through the test cluster as assignmentType - it's not too late.
The presence of "variationType" is how the test runner decides between the /flags and the /bandits endpoints of the relay server (should probably use actions instead). I had considered specifying an endpoint per assignment type and leave this field out altogether, but it makes for a lot of duplicated code.
| "banditLog" => array_map('json_encode', $this->logger->banditLogs) | ||
| ); | ||
| } finally { | ||
| $this->logger->resetLogs(); |
There was a problem hiding this comment.
Why $this-> here and nowhere else we reference $logger?
There was a problem hiding this comment.
clarified the two loggers; one is for this application to log to the console and one is catching Eppo events for the handler to return.
|
|
||
| $app = AppFactory::create(); | ||
|
|
||
| $app->get('/', function (Request $request, Response $response, array $args) { |
There was a problem hiding this comment.
Nice thinking! Could be useful for docker healthcheck too
Eppo Internal:
Notion
Related PRs
Motivation and Context
The SDK Package Testing Cluster is a reusable scaffold for testing each SDK (and a specific version thereof) in a production-like environment using a shared test runner and mock API server. For each deployment variation of each SDK, a "Relay Server" is built in a Docker image capable of running with a specific version of the SDK (by Git tag, branch, or commit). A test runner application dictates to the API server what UFC and Bandit model data to serve and POSTs the test cases to the relay server.
The SDK Relay Server for PHP (SRS4PHP) is a simple application which takes requests from HTTP Posts, passes them to the php SDK and returns the results (and logs) in an HTTP response.
Overview of Changes
build.shto build the relay app with a specific version of the SDKrelease.shto encode the proper Image tagOutstanding work
build.shfile injects the desired version of the SDK at container run time)What is requested of the reviewers