FF-2945 feat: Python SDK #29

rasendubi · 2024-08-21T10:32:47Z

This PR is ready for review.

It's probably best to start review with the announcement to understand main changes: Eppo-exp/eppo-docs#454

rasendubi · 2024-08-21T17:37:51Z

python-sdk/pyproject.toml

+readme = "README.md"
+authors = [{ name = "Eppo", email = "[email protected]" }]
+license = { file = "LICENSE" }
+requires-python = ">=3.8"


☝️ bumping required python version from 3.6 to 3.8

Sounds good, given 3.6 hit end of life two years ago 😬

rasendubi · 2024-08-21T17:38:54Z

python-sdk/pytest.ini

@@ -0,0 +1,3 @@
+[pytest]
+markers =
+    rust_only: mark a test as only passing on Python-on-Rust SDK.


☝️ All tests without this marker pass on both python-on-rust and native python implementations

To run tests against old SDK: 1. Create and activate a virtual for the old SDK. 2. Install old SDK with `pip install -e .` 3. Install pytest 4. Run pytest in this directory with `-k 'not rust_only'`

The one file was growing too big. Split it into multiple files for easier maintenance.

…_initialization()

…t_keys()

- add a common util module - move tests outside of python source (this is recommended by pytest to avoid confusion)

This is to free up space for Configuration type that holds UFC flags and bandits configuration.

…s fixed

- allow setting initial configuration - allow disabling polling

Negative values are already forbidden.

Just a copy from the native SDK.

Breaking changes: - ConfigurationStore::set_configuration() now accepts Arc<Configuration> - Fixed default poll interval to 30 (from 5 minutes)

This helps to avoid breakage in benchmarks and examples.

aarsilv

💪 💪 💪 Awesome stuff! 📈

This is really exciting. I'm not approving because I don't know Python well and Rust at all. Although this makes me want to learn the latter 🤓

Thanks to helpful comments and method/variable names, I think I was able to decently follow. 🙏 Left a bunch of minor comments you can take or leave as you see fit!

aarsilv · 2024-09-11T04:10:42Z

.github/workflows/python.yml

@@ -0,0 +1,280 @@
+# This file is autogenerated by maturin v1.7.0


👀 maturin looks pretty sweet!

aarsilv · 2024-09-11T04:12:34Z

eppo_core/src/attributes.rs

+            if let Ok(s) = value.downcast::<PyString>() {
+                return Ok(AttributeValue::String(s.extract()?));
+            }
+            // In Python, Bool inherits from Int, so it must be checked first here.


Appreciate the helpful comments

aarsilv · 2024-09-11T04:15:52Z

eppo_core/src/poller_thread.rs

@@ -28,7 +28,7 @@ pub struct PollerThreadConfig {

 impl PollerThreadConfig {
    /// Default value for [`PollerThreadConfig::interval`].
-    pub const DEFAULT_POLL_INTERVAL: Duration = Duration::from_secs(5 * 60);
+    pub const DEFAULT_POLL_INTERVAL: Duration = Duration::from_secs(30);


👍 Thanks for conforming to our SDK standard of default 30 second polling

aarsilv · 2024-09-11T04:21:50Z

mock-server/prepare.js

@@ -0,0 +1,48 @@
+// This is a prepare script that is automatically run before "start" command.


Why the change to JS and requirement of node?

This is to make it cross-platform and easier to run in CI. Alpine linux doesn't have bash by default (it has sh) and Windows doesn't have any posix shell by default

aarsilv · 2024-09-11T04:23:48Z

python-sdk/.gitignore

+docs/_build/
+
+# PyCharm
+.idea/


hehe, I don't actually use intellij products. this was auto-generated

aarsilv · 2024-09-11T04:30:30Z

python-sdk/python/eppo_client/_eppo_client.pyi

+        ),
+        default: str,
+    ) -> EvaluationResult: ...
+    def get_bandit_action_details(


aarsilv · 2024-09-11T04:31:13Z

python-sdk/python/eppo_client/_eppo_client.pyi

+            Dict[str, ContextAttributes]
+            | Dict[str, Dict[str, Union[str, int, float, bool, None]]]


For later, we should also support contextless actions. I know we try to steer customers away from this, but I've seen this use case come up on many calls.

Do you mean supporting actions: Dict[str, None] or list[str]? We should be able to add this in the future in a backward-compatible way

aarsilv · 2024-09-11T04:37:11Z

python-sdk/src/client.rs

+        subject_attributes: Attributes,
+        default: Py<PyString>,
+    ) -> PyResult<EvaluationResult> {
+        slf.get().get_assignment_details(


would it make sense for the underlying rust function to have details passed in as boolean like pass in type? Then one less "public" function upstream SDKs need to worry about.

Do you mean a type-level parameter like this?

get_assignment::<WithDetails>(...)

It's possible but is a bit convoluted to implement (requires setting up traits and proxying implementation through trait), so isn't worth it for internal function

aarsilv · 2024-09-11T04:40:55Z

python-sdk/src/configuration.rs

+        PySet::new_bound(
+            py,
+            self.configuration
+                .bandits


minor but we're deprecating the array-valued bandits in favor of object-valued banditReferences. Both have a top-level object though whose keys are the bandit keys, so should be an easy swap.

Sounds good. I shall update the core later. (.bandits here is the whole /bandits response. "bandits" field in config response is handled deeper in the core.)

aarsilv · 2024-09-11T04:42:07Z

python-sdk/tests/test_bandit_result.py

+    assert result.to_string() == "action"
+
+
+# Native python implementation does not convert variation to string.


Interesting.

But yeah in the Eppo application UI, we only allow creating string-valued bandit flags

4.0 extends BanditResult to EvaluationResult so different-typed variations are allowed. BanditResult is now an alias to new EvaluationResult

Something like this:

# old BanditResult: class BanditResult: variation: str action: str | None # new EvaluationResult: class EvaluationResult: variation: Any action: str | None evaluation_details: Any | None

leoromanovsky · 2024-09-12T05:58:15Z

I'm able to install with pip but not able to integrate it into a sample program:

➜  python-sdk git:(feat-python-sdk) ✗ pip install -e .
Obtaining file:///Users/leo/src/rust-sdk/python-sdk
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Building wheels for collected packages: eppo-server-sdk
  Building editable for eppo-server-sdk (pyproject.toml) ... done
  Created wheel for eppo-server-sdk: filename=eppo_server_sdk-4.0.0-cp311-cp311-macosx_11_0_arm64.whl size=4297 sha256=d3076a814895414aff8a1e5b29785242d953393c21c320d91d93054dc412e1cb
  Stored in directory: /private/var/folders/1f/95qsl9p95fz0k6jgh5cgcd3r0000gn/T/pip-ephem-wheel-cache-wsivpflm/wheels/05/a2/8b/febb161210fb0561c324e64b7ff9400edef701dbe616d25850
Successfully built eppo-server-sdk
Installing collected packages: eppo-server-sdk
  Attempting uninstall: eppo-server-sdk
    Found existing installation: eppo-server-sdk 4.0.0
    Uninstalling eppo-server-sdk-4.0.0:
      Successfully uninstalled eppo-server-sdk-4.0.0
Successfully installed eppo-server-sdk-4.0.0

[notice] A new release of pip is available: 24.1.2 -> 24.2
[notice] To update, run: python3.11 -m pip install --upgrade pip

❌

➜  python-sdk git:(feat-python-sdk) ✗ python3 -c "from eppo_client import EppoClient; print(EppoClient)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'eppo_client'

The existing sample program I have is:

import eppo_client
import time
from eppo_client.config import Config
from eppo_client.assignment_logger import AssignmentLogger

# Set this as your Eppo API Key - https://docs.geteppo.com/prerequisites/feature-flagging/randomization-sdk/api-keys
api_key = ""

# Set this one of your Eppo experiment keys
experiment_key = ""

class ExampleAssignmentLogger(AssignmentLogger):
  def log_assignment(self, assignment_event):
    return print("Assigned experiment %s variation %s to subject %s" %(assignment_event["experiment"], assignment_event["variation"], assignment_event["subject"]))

if __name__ == "__main__":
  if not api_key:
    print("Edit example.py to enter your Eppo API key")
    exit(1)
  
  config = Config(api_key=api_key, assignment_logger=ExampleAssignmentLogger())
  client = eppo_client.init(config)
  time.sleep(1) # wait for initialization
  variation = client.get_assignment("my-user", experiment_key)
  if variation is None:
    print("No variation assigned. Check that the experiment is running.")

Are adjustments needed to it or to pyproject.toml ?

rasendubi · 2024-09-12T09:08:57Z

@leoromanovsky that's interesting. The import should work without any changes.

Could you give me the output of the following:

which pip
which python3
python3 --version
ls python/eppo_client

also, does it work with non-editable install (pip install .)?

leoromanovsky · 2024-09-12T16:47:42Z

Could you give me the output of the following:

✅ got it working!

leoromanovsky

Ran it in a test python server and it operated as expected.

Really appreciate all the comments you added and explaining your rationale for decisions.

rasendubi · 2024-09-17T08:26:42Z

Going to merge this, so next PRs take python into account. Will coordinate with you to release it later

rasendubi changed the title ~~feat: Python SDK~~ FF-2945 feat: Python SDK Aug 21, 2024

rasendubi commented Aug 21, 2024

View reviewed changes

rasendubi added 5 commits August 22, 2024 20:37

feat(eppo_core): make PollerThread Sync

185f0d9

feat: implement python-sdk

d6a1f80

test(python): adapt tests to run against eppo_client-3

dbb6ccd

To run tests against old SDK: 1. Create and activate a virtual for the old SDK. 2. Install old SDK with `pip install -e .` 3. Install pytest 4. Run pytest in this directory with `-k 'not rust_only'`

refactor(python): split into multiple files

3c391dd

The one file was growing too big. Split it into multiple files for easier maintenance.

feat(python): add bandits

ce8428b

rasendubi force-pushed the feat-python-sdk branch from b135f45 to ce8428b Compare August 22, 2024 17:37

rasendubi added 12 commits August 22, 2024 20:53

feat(python): log bandit actions

bca541c

feat(python): add non-graceful mode

eba132f

test(python): exclude weakly-typed tests

e1fb6d6

feat(python): add BanditResult.to_string()

7785300

feat(python): add EppoClient.is_initialized() and EppoClient.wait_for…

c3b0cce

…_initialization()

feat(python): add EppoClient.get_flag_keys() and EppoClient.get_bandi…

52ef063

…t_keys()

test(python): refactor tests

b639009

- add a common util module - move tests outside of python source (this is recommended by pytest to avoid confusion)

feat(python): add EppoClient.get_bandit_action_details()

32ee030

refactor(python): rename Config to ClientConfig

f6433ab

This is to free up space for Configuration type that holds UFC flags and bandits configuration.

feat(python): add Configuration

0431834

feat(python): export version string

94ed30b

test(python): add tox configuration

e06b0f6

rasendubi force-pushed the feat-python-sdk branch 9 times, most recently from 4d098a2 to 61d7ebe Compare August 28, 2024 17:10

rasendubi added 2 commits September 4, 2024 16:03

chore(python): echo commands in CI

4e61470

chore(python): disable test on musllinux armv7 until upstream issue i…

3d0b3b9

…s fixed

rasendubi force-pushed the feat-python-sdk branch 3 times, most recently from 88f4b6d to 7f5f777 Compare September 4, 2024 14:00

chore(python): bundle openssl on linux

9ad5e54

rasendubi force-pushed the feat-python-sdk branch from 7f5f777 to 9ad5e54 Compare September 4, 2024 14:10

rasendubi added 2 commits September 9, 2024 15:23

feat(python): restore feature parity with native SDK

d845ccd

- allow setting initial configuration - allow disabling polling

feat(python): forbid 0 for poll_interval_seconds

20c8bf5

Negative values are already forbidden.

rasendubi force-pushed the feat-python-sdk branch from 36d6332 to 4510639 Compare September 9, 2024 13:05

rasendubi added 2 commits September 9, 2024 16:16

feat: add AssignmentCacheLogger

1d57dca

Just a copy from the native SDK.

feat(python): add typing information

92941b4

rasendubi force-pushed the feat-python-sdk branch from 4510639 to 92941b4 Compare September 9, 2024 13:16

fix(python): small typing fixes

c86069b

rasendubi mentioned this pull request Sep 9, 2024

FF-2945 Add announcement for Python SDK 4.0 Eppo-exp/eppo-docs#454

Merged

rasendubi added 3 commits September 9, 2024 20:24

fix: adapt tests to new configuration store API

ca691b3

chore: bump eppo_core to 3.0.0

2b92097

Breaking changes: - ConfigurationStore::set_configuration() now accepts Arc<Configuration> - Fixed default poll interval to 30 (from 5 minutes)

chore(ci): build all targets in CI

890767d

This helps to avoid breakage in benchmarks and examples.

rasendubi requested review from aarsilv and leoromanovsky September 9, 2024 17:32

aarsilv reviewed Sep 11, 2024

View reviewed changes

aarsilv assigned leoromanovsky Sep 11, 2024

docs(python): update README

d6fab8b

leoromanovsky approved these changes Sep 17, 2024

View reviewed changes

rasendubi merged commit 7d6848f into main Sep 17, 2024
24 checks passed

rasendubi deleted the feat-python-sdk branch September 17, 2024 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FF-2945 feat: Python SDK #29

FF-2945 feat: Python SDK #29

rasendubi commented Aug 21, 2024 •

edited

Loading

rasendubi Aug 21, 2024

aarsilv Sep 11, 2024

rasendubi Aug 21, 2024

aarsilv left a comment

aarsilv Sep 11, 2024

aarsilv Sep 11, 2024

aarsilv Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

aarsilv Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

aarsilv Sep 11, 2024

rasendubi Sep 11, 2024

leoromanovsky commented Sep 12, 2024

rasendubi commented Sep 12, 2024 •

edited

Loading

leoromanovsky commented Sep 12, 2024 •

edited

Loading

leoromanovsky left a comment

rasendubi commented Sep 17, 2024

		@@ -0,0 +1,280 @@
		# This file is autogenerated by maturin v1.7.0

		@@ -0,0 +1,48 @@
		// This is a prepare script that is automatically run before "start" command.

		Dict[str, ContextAttributes]
		\| Dict[str, Dict[str, Union[str, int, float, bool, None]]]

		assert result.to_string() == "action"


		# Native python implementation does not convert variation to string.

FF-2945 feat: Python SDK #29

FF-2945 feat: Python SDK #29

Conversation

rasendubi commented Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aarsilv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leoromanovsky commented Sep 12, 2024

rasendubi commented Sep 12, 2024 • edited Loading

leoromanovsky commented Sep 12, 2024 • edited Loading

leoromanovsky left a comment

Choose a reason for hiding this comment

rasendubi commented Sep 17, 2024

rasendubi commented Aug 21, 2024 •

edited

Loading

rasendubi commented Sep 12, 2024 •

edited

Loading

leoromanovsky commented Sep 12, 2024 •

edited

Loading