Skip to content

Conversation

@pauortegathoughtworks
Copy link

@pauortegathoughtworks pauortegathoughtworks commented Jun 8, 2023

PHP SDK

What did you accomplish?

Since the Split Synchronizer documentation recommends using a dedicated Redis database for splits users might need to specify the database they're using when initializing the SDK instance.

$parameters = array(
    'scheme' => 'redis',
    'host' => REDIS_HOST,
    'port' => REDIS_PORT,
    'timeout' => 881,
    'database' => $database
);
$options = array('prefix' => TEST_PREFIX);

$sdkConfig = array(
    'log' => array('adapter' => 'stdout'),
    'cache' => array('adapter' => 'predis', 'parameters' => $parameters, 'options' => $options)
);

//Initializing the SDK instance.
$splitFactory = \SplitIO\Sdk::factory('asdqwe123456', $sdkConfig);
$splitSdk = $splitFactory->client();

While this functionality already exists it is:

a) not documented in Split.io PHP SDK page
b) as far as I can tell, not tested

This PR is aimed at fixing point b), and raising the question on how to fix point a).

How do we test the changes introduced in this PR?

The change is the test: we added a testDatabaseParameterIsPropagatedToPredisClient in SdkClientTest to verify the database parameter is propagated to the Predis client.

Extra Notes

How can we address point a), that is, how can we make it obvious in Split.io PHP SDK page that database is an accepted parameter?

@pauortegathoughtworks pauortegathoughtworks changed the base branch from master to develop June 8, 2023 14:44
@pauortegathoughtworks pauortegathoughtworks marked this pull request as ready for review June 12, 2023 08:46
@pauortegathoughtworks pauortegathoughtworks requested a review from a team as a code owner November 8, 2023 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant