From 53d6f98efd4c6b37c69ef064995333b16828ed18 Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Wed, 27 Apr 2022 20:15:11 -0700 Subject: [PATCH 1/6] add config param --- LICENSE | 222 ++++------------------------------------- eppo_client/client.py | 25 +++++ eppo_client/config.py | 27 +++++ eppo_client/example.py | 2 - test/client_test.py | 10 ++ test/example_test.py | 5 - 6 files changed, 83 insertions(+), 208 deletions(-) create mode 100644 eppo_client/client.py create mode 100644 eppo_client/config.py delete mode 100644 eppo_client/example.py create mode 100644 test/client_test.py delete mode 100644 test/example_test.py diff --git a/LICENSE b/LICENSE index f49a4e1..46db32b 100644 --- a/LICENSE +++ b/LICENSE @@ -1,201 +1,21 @@ - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS - - APPENDIX: How to apply the Apache License to your work. - - To apply the Apache License to your work, attach the following - boilerplate notice, with the fields enclosed by brackets "[]" - replaced with your own identifying information. (Don't include - the brackets!) The text should be enclosed in the appropriate - comment syntax for the file format. We also recommend that a - file or class name and description of purpose be included on the - same "printed page" as the copyright notice for easier - identification within third-party archives. - - Copyright [yyyy] [name of copyright owner] - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. \ No newline at end of file +MIT License + +Copyright (c) 2022 Eppo Data, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. \ No newline at end of file diff --git a/eppo_client/client.py b/eppo_client/client.py new file mode 100644 index 0000000..a02f466 --- /dev/null +++ b/eppo_client/client.py @@ -0,0 +1,25 @@ +from typing import Optional +from eppo_client.config import Config + + +class EppoClient: + """ + The client should be initialized at application startup as a singleton; use the same client instance for the lifetime of the application. + Use the :func:`eppo_client.get()` method to get a shared instance of the client. + """ + + def __init__(self, config: Config) -> None: + """ + :param config: SDK configuration params including api key + """ + config._validate() + self.__config = config + + def assign(self, subject: str, flag: str) -> Optional[str]: + """Maps a subject to a variation for a given experiment + Returns None if the subject is not part of the experiment sample. + + :param subject: an entity ID, e.g. userId + :param flag: an experiment identifier + """ + return None diff --git a/eppo_client/config.py b/eppo_client/config.py new file mode 100644 index 0000000..bb44a95 --- /dev/null +++ b/eppo_client/config.py @@ -0,0 +1,27 @@ +from typing import Optional + + +class Config: + """ + Configurations passed to :func:`eppo_client.set_config()` to initialize a client instance. + """ + + def __init__(self, api_key: str, base_url: str = "https://eppo.cloud/api"): + """ + :param api_key: Eppo API key + :param base_url: Base URL of the Eppo API. Clients should use the default setting in most cases. + """ + self.__api_key = api_key + self.__base_url = base_url + + @property + def api_key(self) -> Optional[str]: + return self.__api_key + + @property + def base_url(self) -> str: + return self.__base_url + + def _validate(self): + if self.api_key is None or self.api_key == "": + raise ValueError("Invalid configuration: api_key is required") diff --git a/eppo_client/example.py b/eppo_client/example.py deleted file mode 100644 index c929f88..0000000 --- a/eppo_client/example.py +++ /dev/null @@ -1,2 +0,0 @@ -def add_one(number): - return number + 1 diff --git a/test/client_test.py b/test/client_test.py new file mode 100644 index 0000000..80b5203 --- /dev/null +++ b/test/client_test.py @@ -0,0 +1,10 @@ +import pytest +from eppo_client.client import EppoClient +from eppo_client.config import Config + + +def test_blank_api_key(): + with pytest.raises(Exception) as exc_info: + config = Config("") + EppoClient(config=config) + assert exc_info.value.args[0] == "Invalid configuration: api_key is required" diff --git a/test/example_test.py b/test/example_test.py deleted file mode 100644 index 1be1d20..0000000 --- a/test/example_test.py +++ /dev/null @@ -1,5 +0,0 @@ -from eppo_client import example - - -def test_example(): - assert example.add_one(3) == 4 From 65ca85cd04d7c47d5b213623352f82405e0fda57 Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Sat, 30 Apr 2022 14:38:07 -0700 Subject: [PATCH 2/6] add assignment test --- .gitignore | 1 + eppo_client/client.py | 46 +++++++++++--- eppo_client/config.py | 29 +++------ eppo_client/configuration_requestor.py | 42 +++++++++++++ eppo_client/shard.py | 23 +++++++ eppo_client/validation.py | 3 + requirements-test.txt | 3 +- requirements.txt | 1 + test/client_test.py | 85 ++++++++++++++++++++++++-- test/conftest.py | 23 +++++++ tox.ini | 2 +- 11 files changed, 222 insertions(+), 36 deletions(-) create mode 100644 eppo_client/configuration_requestor.py create mode 100644 eppo_client/shard.py create mode 100644 eppo_client/validation.py create mode 100644 test/conftest.py diff --git a/.gitignore b/.gitignore index 6446e0e..23793a7 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ dist/ .env __pycache__ .tox/ +test-data \ No newline at end of file diff --git a/eppo_client/client.py b/eppo_client/client.py index a02f466..860a401 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,5 +1,10 @@ from typing import Optional -from eppo_client.config import Config +from eppo_client.configuration_requestor import ( + ExperimentConfigurationDto, + ExperimentConfigurationRequestor, +) +from eppo_client.shard import get_shard, is_in_shard_range +from eppo_client.validation import validate_not_blank class EppoClient: @@ -8,12 +13,8 @@ class EppoClient: Use the :func:`eppo_client.get()` method to get a shared instance of the client. """ - def __init__(self, config: Config) -> None: - """ - :param config: SDK configuration params including api key - """ - config._validate() - self.__config = config + def __init__(self, config_requestor: ExperimentConfigurationRequestor) -> None: + self.__config_requestor = config_requestor def assign(self, subject: str, flag: str) -> Optional[str]: """Maps a subject to a variation for a given experiment @@ -22,4 +23,33 @@ def assign(self, subject: str, flag: str) -> Optional[str]: :param subject: an entity ID, e.g. userId :param flag: an experiment identifier """ - return None + validate_not_blank("subject", subject) + validate_not_blank("flag", flag) + experiment_config = self.__config_requestor.get_configuration(flag) + if ( + experiment_config == None + or not experiment_config.enabled + or not self.is_in_experiment_sample(subject, flag, experiment_config) + ): + return None + shard = get_shard( + "assignment-{}-{}".format(subject, flag), experiment_config.subjectShards + ) + return next( + ( + variation.name + for variation in experiment_config.variations + if is_in_shard_range(shard, variation.shardRange) + ), + None, + ) + + def is_in_experiment_sample( + self, subject: str, flag: str, experiment_config: ExperimentConfigurationDto + ): + shard = get_shard( + "exposure-{}-{}".format(subject, flag), experiment_config.subjectShards + ) + return ( + shard <= experiment_config.percentExposure * experiment_config.subjectShards + ) diff --git a/eppo_client/config.py b/eppo_client/config.py index bb44a95..2461497 100644 --- a/eppo_client/config.py +++ b/eppo_client/config.py @@ -1,27 +1,12 @@ -from typing import Optional +from dataclasses import dataclass +from eppo_client.validation import validate_not_blank -class Config: - """ - Configurations passed to :func:`eppo_client.set_config()` to initialize a client instance. - """ - - def __init__(self, api_key: str, base_url: str = "https://eppo.cloud/api"): - """ - :param api_key: Eppo API key - :param base_url: Base URL of the Eppo API. Clients should use the default setting in most cases. - """ - self.__api_key = api_key - self.__base_url = base_url - @property - def api_key(self) -> Optional[str]: - return self.__api_key - - @property - def base_url(self) -> str: - return self.__base_url +@dataclass +class Config: + api_key: str + base_url: str = "https://eppo.cloud/api" def _validate(self): - if self.api_key is None or self.api_key == "": - raise ValueError("Invalid configuration: api_key is required") + validate_not_blank("api_key", self.api_key) diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py new file mode 100644 index 0000000..ef1c9f6 --- /dev/null +++ b/eppo_client/configuration_requestor.py @@ -0,0 +1,42 @@ +from dataclasses import dataclass +import json + +from eppo_client.shard import ShardRange + + +@dataclass +class VariationDto: + name: str + shardRange: ShardRange + + @staticmethod + def from_dict(variation_dict): + variation_dict["shardRange"] = ShardRange.from_dict( + variation_dict["shardRange"] + ) + return VariationDto(**variation_dict) + + +@dataclass +class ExperimentConfigurationDto: + subjectShards: int + percentExposure: float + enabled: bool + variations: list[VariationDto] + name: str + + @staticmethod + def from_json(json_str: str): + json_dict = json.load(json_str) + variations = [ + VariationDto.from_dict(variation_dict) + for variation_dict in json_dict["variations"] + ] + json_dict["variations"] = variations + return ExperimentConfigurationDto(**json_dict) + + +class ExperimentConfigurationRequestor: + def get_configuration(flag: str) -> ExperimentConfigurationDto: + # TODO: implement this method + return None diff --git a/eppo_client/shard.py b/eppo_client/shard.py new file mode 100644 index 0000000..aac622a --- /dev/null +++ b/eppo_client/shard.py @@ -0,0 +1,23 @@ +from dataclasses import dataclass +import hashlib + + +def get_shard(input: str, subject_shards: int): + hash_output = hashlib.md5(input.encode("utf-8")).hexdigest() + # get the first 4 bytes of the md5 hex string and parse it using base 16 + # (8 hex characters represent 4 bytes, e.g. 0xffffffff represents the max 4-byte integer) + int_from_hash = int(hash_output[0:8], 16) + return int_from_hash % subject_shards + + +@dataclass +class ShardRange: + start: int + end: int + + def from_dict(range_dict): + return ShardRange(**range_dict) + + +def is_in_shard_range(shard: int, range: ShardRange) -> bool: + return shard >= range.start and shard < range.end diff --git a/eppo_client/validation.py b/eppo_client/validation.py new file mode 100644 index 0000000..2095028 --- /dev/null +++ b/eppo_client/validation.py @@ -0,0 +1,3 @@ +def validate_not_blank(field_name: str, field_value: str): + if field_value is None or field_value == "": + raise ValueError("Invalid value for {}: cannot be blank".format(field_name)) diff --git a/requirements-test.txt b/requirements-test.txt index 32ec8d6..51ee95f 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,3 +1,4 @@ tox pytest -mypy \ No newline at end of file +mypy +google-cloud-storage \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index e69de29..eea1811 100644 --- a/requirements.txt +++ b/requirements.txt @@ -0,0 +1 @@ +dataclasses \ No newline at end of file diff --git a/test/client_test.py b/test/client_test.py index 80b5203..cdc0b96 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -1,10 +1,87 @@ +from dataclasses import dataclass +import json, os +from unittest.mock import patch import pytest from eppo_client.client import EppoClient from eppo_client.config import Config +from eppo_client.configuration_requestor import ( + ExperimentConfigurationDto, + ExperimentConfigurationRequestor, + VariationDto, +) +from eppo_client.shard import ShardRange +mock_api_key = "mock-api-key" -def test_blank_api_key(): + +@dataclass +class AssignmentTestCase: + experiment: str + percentExposure: float + variations: list[VariationDto] + subjects: list[str] + expectedAssignments: list[str] + + +test_data = [] +for file_name in [file for file in os.listdir("test/test-data/assignment")]: + with open("test/test-data/assignment/{}".format(file_name)) as test_case_json: + test_case_dict = json.load(test_case_json) + variations = [ + VariationDto.from_dict(variation_dict) + for variation_dict in test_case_dict["variations"] + ] + test_case_dict["variations"] = variations + test_data.append(AssignmentTestCase(**test_case_dict)) + + +def test_assign_blank_experiment(): + client = EppoClient(config_requestor=ExperimentConfigurationRequestor()) + with pytest.raises(Exception) as exc_info: + client.assign("subject-1", "") + assert exc_info.value.args[0] == "Invalid value for flag: cannot be blank" + + +def test_assign_blank_subject(): + client = EppoClient(config_requestor=ExperimentConfigurationRequestor()) with pytest.raises(Exception) as exc_info: - config = Config("") - EppoClient(config=config) - assert exc_info.value.args[0] == "Invalid configuration: api_key is required" + client.assign("", "experiment-1") + assert exc_info.value.args[0] == "Invalid value for subject: cannot be blank" + + +@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") +def test_assign_subject_not_in_sample(mock_config_requestor): + mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( + subjectShards=10000, + percentExposure=0, + enabled=True, + variations=[ + VariationDto(name="control", shardRange=ShardRange(start=0, end=100)) + ], + name="recommendation_algo", + ) + client = EppoClient(config_requestor=mock_config_requestor) + assert client.assign("user-1", "flag-1") == None + + +@pytest.mark.parametrize("test_case", test_data) +def test_assign_subject_in_sample(test_case): + print("---- Test case for {} Experiment".format(test_case.experiment)) + with patch( + "eppo_client.configuration_requestor.ExperimentConfigurationRequestor" + ) as mock_config_requestor: + mock_config_requestor.get_configuration.return_value = ( + ExperimentConfigurationDto( + subjectShards=10000, + percentExposure=test_case.percentExposure, + enabled=True, + variations=test_case.variations, + name=test_case.experiment, + ) + ) + client = EppoClient(config_requestor=mock_config_requestor) + assignments = [ + client.assign(subject, test_case.experiment) + for subject in test_case.subjects + ] + assert assignments == test_case.expectedAssignments diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 0000000..1d7e907 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,23 @@ +import os +from google.cloud import storage + +TEST_DATA_DIR = "test/test-data/assignment" + + +def download_assignment_test_data(): + """Downloads assignment test data from a cloud storage bucket + The same test data is used by all Eppo SDKs. + """ + storage_client = storage.Client.create_anonymous_client() + bucket = storage_client.bucket("sdk-test-data") + blobs = bucket.list_blobs(prefix="assignment/test-case") + for blob in blobs: + blob.download_to_filename( + "{}/{}".format(TEST_DATA_DIR, blob.name.split("/")[1]) + ) + + +def pytest_configure(config): + if not os.path.exists(TEST_DATA_DIR): + os.makedirs(TEST_DATA_DIR) + download_assignment_test_data() diff --git a/tox.ini b/tox.ini index c4f5d64..144a179 100644 --- a/tox.ini +++ b/tox.ini @@ -4,8 +4,8 @@ isolated_build = True [testenv] deps = - -rrequirements.txt -rrequirements-test.txt + -rrequirements.txt commands = python -m pytest {posargs} From 6e7270b60d1035874fdc44c147e8930033f774c2 Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Sat, 30 Apr 2022 16:32:17 -0700 Subject: [PATCH 3/6] fix linter errors --- eppo_client/client.py | 7 +------ eppo_client/configuration_requestor.py | 14 ++------------ test/client_test.py | 6 +++--- test/conftest.py | 2 +- 4 files changed, 7 insertions(+), 22 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 860a401..03da2d9 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -8,11 +8,6 @@ class EppoClient: - """ - The client should be initialized at application startup as a singleton; use the same client instance for the lifetime of the application. - Use the :func:`eppo_client.get()` method to get a shared instance of the client. - """ - def __init__(self, config_requestor: ExperimentConfigurationRequestor) -> None: self.__config_requestor = config_requestor @@ -27,7 +22,7 @@ def assign(self, subject: str, flag: str) -> Optional[str]: validate_not_blank("flag", flag) experiment_config = self.__config_requestor.get_configuration(flag) if ( - experiment_config == None + experiment_config is None or not experiment_config.enabled or not self.is_in_experiment_sample(subject, flag, experiment_config) ): diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index ef1c9f6..8a3da7f 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -import json +from typing import Optional from eppo_client.shard import ShardRange @@ -25,18 +25,8 @@ class ExperimentConfigurationDto: variations: list[VariationDto] name: str - @staticmethod - def from_json(json_str: str): - json_dict = json.load(json_str) - variations = [ - VariationDto.from_dict(variation_dict) - for variation_dict in json_dict["variations"] - ] - json_dict["variations"] = variations - return ExperimentConfigurationDto(**json_dict) - class ExperimentConfigurationRequestor: - def get_configuration(flag: str) -> ExperimentConfigurationDto: + def get_configuration(self, flag: str) -> Optional[ExperimentConfigurationDto]: # TODO: implement this method return None diff --git a/test/client_test.py b/test/client_test.py index cdc0b96..6cf6a4b 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -1,9 +1,9 @@ from dataclasses import dataclass -import json, os +import json +import os from unittest.mock import patch import pytest from eppo_client.client import EppoClient -from eppo_client.config import Config from eppo_client.configuration_requestor import ( ExperimentConfigurationDto, ExperimentConfigurationRequestor, @@ -61,7 +61,7 @@ def test_assign_subject_not_in_sample(mock_config_requestor): name="recommendation_algo", ) client = EppoClient(config_requestor=mock_config_requestor) - assert client.assign("user-1", "flag-1") == None + assert client.assign("user-1", "flag-1") is None @pytest.mark.parametrize("test_case", test_data) diff --git a/test/conftest.py b/test/conftest.py index 1d7e907..e2e2d5a 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,5 +1,5 @@ import os -from google.cloud import storage +from google.cloud import storage # type: ignore TEST_DATA_DIR = "test/test-data/assignment" From 65f55bccd557730c4062a795743c5857d45754bf Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Sat, 30 Apr 2022 16:47:22 -0700 Subject: [PATCH 4/6] fix linter error --- eppo_client/configuration_requestor.py | 4 ++-- test/client_test.py | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index 8a3da7f..af2df75 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Optional +from typing import List, Optional from eppo_client.shard import ShardRange @@ -22,7 +22,7 @@ class ExperimentConfigurationDto: subjectShards: int percentExposure: float enabled: bool - variations: list[VariationDto] + variations: List[VariationDto] name: str diff --git a/test/client_test.py b/test/client_test.py index 6cf6a4b..cd29e06 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -1,6 +1,7 @@ from dataclasses import dataclass import json import os +from typing import List from unittest.mock import patch import pytest from eppo_client.client import EppoClient @@ -18,9 +19,9 @@ class AssignmentTestCase: experiment: str percentExposure: float - variations: list[VariationDto] - subjects: list[str] - expectedAssignments: list[str] + variations: List[VariationDto] + subjects: List[str] + expectedAssignments: List[str] test_data = [] From b24f94e86a7fd9d1e3358a44e03a8719060e6c30 Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Mon, 2 May 2022 17:19:24 -0700 Subject: [PATCH 5/6] use pydantic for models --- eppo_client/base_model.py | 14 ++++++++++++++ eppo_client/client.py | 9 +++++---- eppo_client/config.py | 5 ++--- eppo_client/configuration_requestor.py | 21 ++++++--------------- eppo_client/shard.py | 9 +++------ requirements.txt | 2 +- test/client_test.py | 17 ++++++++--------- 7 files changed, 39 insertions(+), 38 deletions(-) create mode 100644 eppo_client/base_model.py diff --git a/eppo_client/base_model.py b/eppo_client/base_model.py new file mode 100644 index 0000000..1541f7a --- /dev/null +++ b/eppo_client/base_model.py @@ -0,0 +1,14 @@ +from pydantic import BaseModel + + +def to_camel(s: str): + words = s.split("_") + if len(words) > 1: + return words[0] + "".join([w.capitalize() for w in words[1:]]) + return words[0] + + +class SdkBaseModel(BaseModel): + class Config: + alias_generator = to_camel + allow_population_by_field_name = True diff --git a/eppo_client/client.py b/eppo_client/client.py index 03da2d9..ce0f2be 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -28,13 +28,13 @@ def assign(self, subject: str, flag: str) -> Optional[str]: ): return None shard = get_shard( - "assignment-{}-{}".format(subject, flag), experiment_config.subjectShards + "assignment-{}-{}".format(subject, flag), experiment_config.subject_shards ) return next( ( variation.name for variation in experiment_config.variations - if is_in_shard_range(shard, variation.shardRange) + if is_in_shard_range(shard, variation.shard_range) ), None, ) @@ -43,8 +43,9 @@ def is_in_experiment_sample( self, subject: str, flag: str, experiment_config: ExperimentConfigurationDto ): shard = get_shard( - "exposure-{}-{}".format(subject, flag), experiment_config.subjectShards + "exposure-{}-{}".format(subject, flag), experiment_config.subject_shards ) return ( - shard <= experiment_config.percentExposure * experiment_config.subjectShards + shard + <= experiment_config.percent_exposure * experiment_config.subject_shards ) diff --git a/eppo_client/config.py b/eppo_client/config.py index 2461497..c82bad1 100644 --- a/eppo_client/config.py +++ b/eppo_client/config.py @@ -1,10 +1,9 @@ -from dataclasses import dataclass +from eppo_client.base_model import SdkBaseModel from eppo_client.validation import validate_not_blank -@dataclass -class Config: +class Config(SdkBaseModel): api_key: str base_url: str = "https://eppo.cloud/api" diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index af2df75..f6e0133 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -1,26 +1,17 @@ -from dataclasses import dataclass from typing import List, Optional +from eppo_client.base_model import SdkBaseModel from eppo_client.shard import ShardRange -@dataclass -class VariationDto: +class VariationDto(SdkBaseModel): name: str - shardRange: ShardRange + shard_range: ShardRange - @staticmethod - def from_dict(variation_dict): - variation_dict["shardRange"] = ShardRange.from_dict( - variation_dict["shardRange"] - ) - return VariationDto(**variation_dict) - -@dataclass -class ExperimentConfigurationDto: - subjectShards: int - percentExposure: float +class ExperimentConfigurationDto(SdkBaseModel): + subject_shards: int + percent_exposure: float enabled: bool variations: List[VariationDto] name: str diff --git a/eppo_client/shard.py b/eppo_client/shard.py index aac622a..5183ba5 100644 --- a/eppo_client/shard.py +++ b/eppo_client/shard.py @@ -1,6 +1,7 @@ -from dataclasses import dataclass import hashlib +from eppo_client.base_model import SdkBaseModel + def get_shard(input: str, subject_shards: int): hash_output = hashlib.md5(input.encode("utf-8")).hexdigest() @@ -10,14 +11,10 @@ def get_shard(input: str, subject_shards: int): return int_from_hash % subject_shards -@dataclass -class ShardRange: +class ShardRange(SdkBaseModel): start: int end: int - def from_dict(range_dict): - return ShardRange(**range_dict) - def is_in_shard_range(shard: int, range: ShardRange) -> bool: return shard >= range.start and shard < range.end diff --git a/requirements.txt b/requirements.txt index eea1811..59cc1e9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1 @@ -dataclasses \ No newline at end of file +pydantic \ No newline at end of file diff --git a/test/client_test.py b/test/client_test.py index cd29e06..9bc2736 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -1,9 +1,9 @@ -from dataclasses import dataclass import json import os -from typing import List +from typing import List, Optional from unittest.mock import patch import pytest +from eppo_client.base_model import SdkBaseModel from eppo_client.client import EppoClient from eppo_client.configuration_requestor import ( ExperimentConfigurationDto, @@ -15,13 +15,12 @@ mock_api_key = "mock-api-key" -@dataclass -class AssignmentTestCase: +class AssignmentTestCase(SdkBaseModel): experiment: str - percentExposure: float + percent_exposure: float variations: List[VariationDto] subjects: List[str] - expectedAssignments: List[str] + expected_assignments: List[Optional[str]] test_data = [] @@ -29,7 +28,7 @@ class AssignmentTestCase: with open("test/test-data/assignment/{}".format(file_name)) as test_case_json: test_case_dict = json.load(test_case_json) variations = [ - VariationDto.from_dict(variation_dict) + VariationDto(**variation_dict) for variation_dict in test_case_dict["variations"] ] test_case_dict["variations"] = variations @@ -74,7 +73,7 @@ def test_assign_subject_in_sample(test_case): mock_config_requestor.get_configuration.return_value = ( ExperimentConfigurationDto( subjectShards=10000, - percentExposure=test_case.percentExposure, + percentExposure=test_case.percent_exposure, enabled=True, variations=test_case.variations, name=test_case.experiment, @@ -85,4 +84,4 @@ def test_assign_subject_in_sample(test_case): client.assign(subject, test_case.experiment) for subject in test_case.subjects ] - assert assignments == test_case.expectedAssignments + assert assignments == test_case.expected_assignments From f0024047bddf50ad6259d293b7bb417b4a513fe0 Mon Sep 17 00:00:00 2001 From: Peter Loomis Date: Tue, 3 May 2022 15:12:51 -0700 Subject: [PATCH 6/6] rename flag -> experiment_key --- eppo_client/client.py | 23 +++++++++++++++-------- eppo_client/configuration_requestor.py | 4 +++- test/client_test.py | 4 ++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index ce0f2be..cc3ef4a 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -11,24 +11,27 @@ class EppoClient: def __init__(self, config_requestor: ExperimentConfigurationRequestor) -> None: self.__config_requestor = config_requestor - def assign(self, subject: str, flag: str) -> Optional[str]: + def assign(self, subject: str, experiment_key: str) -> Optional[str]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. :param subject: an entity ID, e.g. userId - :param flag: an experiment identifier + :param experiment_key: an experiment identifier """ validate_not_blank("subject", subject) - validate_not_blank("flag", flag) - experiment_config = self.__config_requestor.get_configuration(flag) + validate_not_blank("experiment_key", experiment_key) + experiment_config = self.__config_requestor.get_configuration(experiment_key) if ( experiment_config is None or not experiment_config.enabled - or not self.is_in_experiment_sample(subject, flag, experiment_config) + or not self.is_in_experiment_sample( + subject, experiment_key, experiment_config + ) ): return None shard = get_shard( - "assignment-{}-{}".format(subject, flag), experiment_config.subject_shards + "assignment-{}-{}".format(subject, experiment_key), + experiment_config.subject_shards, ) return next( ( @@ -40,10 +43,14 @@ def assign(self, subject: str, flag: str) -> Optional[str]: ) def is_in_experiment_sample( - self, subject: str, flag: str, experiment_config: ExperimentConfigurationDto + self, + subject: str, + experiment_key: str, + experiment_config: ExperimentConfigurationDto, ): shard = get_shard( - "exposure-{}-{}".format(subject, flag), experiment_config.subject_shards + "exposure-{}-{}".format(subject, experiment_key), + experiment_config.subject_shards, ) return ( shard diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index f6e0133..5aa2686 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -18,6 +18,8 @@ class ExperimentConfigurationDto(SdkBaseModel): class ExperimentConfigurationRequestor: - def get_configuration(self, flag: str) -> Optional[ExperimentConfigurationDto]: + def get_configuration( + self, experiment_key: str + ) -> Optional[ExperimentConfigurationDto]: # TODO: implement this method return None diff --git a/test/client_test.py b/test/client_test.py index 9bc2736..8b2486e 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -39,7 +39,7 @@ def test_assign_blank_experiment(): client = EppoClient(config_requestor=ExperimentConfigurationRequestor()) with pytest.raises(Exception) as exc_info: client.assign("subject-1", "") - assert exc_info.value.args[0] == "Invalid value for flag: cannot be blank" + assert exc_info.value.args[0] == "Invalid value for experiment_key: cannot be blank" def test_assign_blank_subject(): @@ -61,7 +61,7 @@ def test_assign_subject_not_in_sample(mock_config_requestor): name="recommendation_algo", ) client = EppoClient(config_requestor=mock_config_requestor) - assert client.assign("user-1", "flag-1") is None + assert client.assign("user-1", "experiment-key-1") is None @pytest.mark.parametrize("test_case", test_data)