[FSSDK-11458] Python - Add SDK Multi-Region Support for Data Hosting by esrakartalOpt · Pull Request #459 · optimizely/python-sdk

esrakartalOpt · 2025-07-02T11:27:28Z

Summary

Add Multi-Region Support for Data Hosting

Test plan

Created and added new test cases

Issues

FSSDK-11458

…o esra/FSSDK-11458_eu_hosting

raju-opti

few small comments

raju-opti · 2025-07-25T18:09:36Z

tests/test_user_event_factory.py

+        )
+
+        self.assertEqual(self.project_config.region, impression_event.event_context.region)
+        self.assertEqual('EU', impression_event.event_context.region)


can we also add a test for create_contervsion_event?

raju-opti · 2025-07-25T18:12:06Z

tests/test_event_processor.py


    def dispatch_event(self, log_event):
-        if log_event.http_verb == 'POST' and log_event.url == EventFactory.EVENT_ENDPOINT:
+        if log_event.url in EventFactory.EVENT_ENDPOINTS.values():


the log_event.http_verb == 'POST' condition got removed. Can we keep that check?

raju-opti · 2025-07-25T18:15:20Z

tests/test_event_factory.py

+            expected_params,
+            EventFactory.HTTP_VERB,
+            EventFactory.HTTP_HEADERS,
        )


can we add a test to check that create_log_event works correctly with EU region?

Copilot

Pull Request Overview

This PR adds multi-region support for data hosting in the Optimizely Python SDK, allowing events to be sent to region-specific endpoints (US or EU) based on project configuration.

Added Region enum with US and EU values to project configuration
Updated event factories and builders to use region-specific endpoints
Enhanced event context to include region information

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
optimizely/project_config.py	Added Region enum and region configuration parsing with US as default
optimizely/event/user_event.py	Updated EventContext to accept and store region parameter
optimizely/event/user_event_factory.py	Modified factory methods to pass region to EventContext
optimizely/event/event_factory.py	Replaced single endpoint with region-specific endpoint mapping
optimizely/event_builder.py	Updated to use region-specific URLs for event creation
tests/test_config.py	Added tests for region configuration parsing
tests/test_user_event_factory.py	Added tests for region handling in event creation
tests/test_event_factory.py	Updated tests to use new endpoint structure
tests/test_event_builder.py	Updated tests for region-specific event building
tests/test_event_processor.py	Updated test to handle multiple endpoints

Copilot · 2025-07-31T16:33:02Z

optimizely/event/user_event.py

        self.client_name = CLIENT_NAME
        self.client_version = version.__version__
        self.anonymize_ip = anonymize_ip
+        self.region = region or 'US'


The fallback logic uses a string literal 'US' instead of the Region enum. Consider using self.region = region or Region.US for consistency with the type system.

Suggested change

self.region = region or 'US'

self.region = region or Region.US

I second htis use. If yuo have Region class somewhere, then we should call country attribute on the Region object.

Copilot · 2025-07-31T16:33:03Z

optimizely/event_builder.py

+        region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'
+        events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])


The fallback to string literal 'US' should use Region.US.value for consistency. Also, this hasattr check suggests uncertainty about the type - consider using isinstance(project_config.region, Region) for more explicit type checking.

Suggested change

region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'

events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])

region = project_config.region.value if isinstance(project_config.region, Region) else Region.US.value

events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS[Region.US.value])

Again, I agree with copilot lol

Copilot · 2025-07-31T16:33:03Z

optimizely/event_builder.py

+        region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'
+        events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])


Same issue as line 272 - the fallback to string literal 'US' should use Region.US.value for consistency, and the hasattr check could be replaced with more explicit type checking.

Suggested change

region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'

events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])

region = project_config.region.value if isinstance(project_config.region, enums.Region) else enums.Region.US.value

events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS[enums.Region.US.value])

Copilot · 2025-07-31T16:33:03Z

tests/test_event_builder.py

                '111129',
                'test_user',
-                {'do_you_know_me': 'test_value'},
+                {'do_you_know_me': 'test_value'}


[nitpick] Removed trailing comma in function call. While this change works, trailing commas in multi-line function calls are often preferred for easier version control diffs.

Suggested change

{'do_you_know_me': 'test_value'}

{'do_you_know_me': 'test_value',}

Copilot · 2025-07-31T16:33:04Z

tests/test_event_builder.py

                '111129',
                'test_user',
-                attributes,
+                attributes


[nitpick] Removed trailing comma in function call. While this change works, trailing commas in multi-line function calls are often preferred for easier version control diffs.

Suggested change

attributes

attributes,

Copilot · 2025-07-31T16:33:04Z

tests/test_event_builder.py

                '111129',
                'test_user',
-                {'$opt_user_agent': 'Edge'},
+                {'$opt_user_agent': 'Edge'}


[nitpick] Removed trailing comma in function call. While this change works, trailing commas in multi-line function calls are often preferred for easier version control diffs.

Suggested change

{'$opt_user_agent': 'Edge'}

{'$opt_user_agent': 'Edge',}

pvcraven

If we specify a region that doesn't exist, what happens? Does 'ZZ' fall back to 'US'? I think this is what happens. I'd like to see tests verifying what happens in this case. (Do we need to log something if we are passed an invalid region?)

jaeopt

Looks good! We can consider string-type region for simple.

jaeopt · 2025-07-31T16:42:50Z

optimizely/event/event_factory.py

+        region = user_context.region
+        if hasattr(region, 'value'):
+            region_str = region.value
+        elif region is None:
+            region_str = 'US'  # Default to US
+        else:
+            region_str = str(region)


We can consider using a simple string type for region. The region will be handled in type-safe way when building into datafile. It looks like fallback to "US" for safety is good enough?

@jaeopt by string type do you mean using Region.US instead of string 'US'?
That's what I have been suggesting further down the code. Just we don't contradict each other.

I mean we get string-type of region from datafile, and we can pass it down all the way to EventDispatcher to determine the URL (with fallback to US). The source of this region is datafile. Not much value of this enum-based type checking. It may be risky if you use defined enum set for strong-typed SDKs when we add new countries in the server.

jaeopt · 2025-07-31T16:44:50Z

optimizely/project_config.py

+        region_value = config.get('region')
+        self.region: Region
+        if region_value == Region.EU.value:
+            self.region = Region.EU
+        else:
+            self.region = Region.US


Another area we can get benefits with string type. No need to change the code when we add countries.

Mat001 · 2025-07-31T16:52:49Z

optimizely/event/user_event.py

        self.client_name = CLIENT_NAME
        self.client_version = version.__version__
        self.anonymize_ip = anonymize_ip
+        self.region = region or 'US'


I second htis use. If yuo have Region class somewhere, then we should call country attribute on the Region object.

Mat001 · 2025-07-31T16:53:44Z

optimizely/event_builder.py

+        region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'
+        events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])


Again, I agree with copilot lol

Mat001 · 2025-07-31T16:56:06Z

optimizely/event/event_factory.py

+        region = user_context.region
+        if hasattr(region, 'value'):
+            region_str = region.value
+        elif region is None:
+            region_str = 'US'  # Default to US
+        else:
+            region_str = str(region)


@jaeopt by string type do you mean using Region.US instead of string 'US'?
That's what I have been suggesting further down the code. Just we don't contradict each other.

esrakartalOpt · 2025-07-31T18:52:40Z

If we specify a region that doesn't exist, what happens? Does 'ZZ' fall back to 'US'? I think this is what happens. I'd like to see tests verifying what happens in this case. (Do we need to log something if we are passed an invalid region?)

@pvcraven Locally, I have created a test case as below for invalid region. It returned US endpoint.

def test_create_conversion_event_with_invalid_region(self):
        """ Test that create_conversion_event creates Event object
    with right params when no attributes are provided. """

        expected_params = {
            'account_id': '12001',
            'project_id': '111001',
            'visitors': [
                {
                    'visitor_id': 'test_user',
                    'attributes': [],
                    'snapshots': [
                        {
                            'events': [
                                {
                                    'timestamp': 42123,
                                    'entity_id': '111095',
                                    'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
                                    'key': 'test_event',
                                }
                            ]
                        }
                    ],
                }
            ],
            'client_name': 'python-sdk',
            'client_version': version.__version__,
            'enrich_decisions': True,
            'anonymize_ip': False,
            'revision': '42',
        }

        with mock.patch.object(self.project_config, 'region', new='ZZ'), mock.patch(
            'time.time', return_value=42.123
        ), mock.patch(
            'uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'
        ):
            event_obj = self.event_builder.create_conversion_event(
                self.project_config, 'test_event', 'test_user', None, None
            )
        self._validate_event_object(
            event_obj,
            event_builder.EventBuilder.EVENTS_URLS.get('US'),
            expected_params,
            event_builder.EventBuilder.HTTP_VERB,
            event_builder.EventBuilder.HTTP_HEADERS,
        )

Mat001

ok

jaeopt

LGTM

[FSSDK-11458] Python - Add SDK Multi-Region Support for Data Hosting

d9e8f83

optimizely-codereview-bot assigned esrakartalOpt Jul 2, 2025

esrakartalOpt added 28 commits July 2, 2025 15:30

Fix lint issues

0866962

Fix lint issue

5a0e19d

Fix errors

730fede

Add region on base test

47ccb94

Update Region as enum

a7d74ac

Delete blank lines

8a65e0f

Fix lint issue

24d2fcd

Fix lint issue

ba5beee

Fix test errors

17c9280

Add enum

155ab25

Fix region value

ba049b6

Fix type issue

a777fa0

Correct the event url

c1f1693

Add region in params

6de0889

Fix region param

83afabe

Add region to create

158aab3

Fix test cases

0a0cdef

Fix test cases

f68d678

Fix lint

81cd042

Add region

9d2c892

Fix test cases

3c1332f

Fix tests

11523cf

Cast to string

1af68db

Fix tests

7a09f1d

Merge branch 'master' of https://github.com/optimizely/python-sdk int…

0b1e83f

…o esra/FSSDK-11458_eu_hosting

Fix order

e07c1b4

Remove unnecessary region implementation

1d61988

Fix lint issues

e24a34e

esrakartalOpt added 2 commits July 24, 2025 12:31

Fix test cases

80365fa

Fix error

174a7d0

raju-opti requested changes Jul 25, 2025

View reviewed changes

Implemented reviews and added new tests

f83865f

esrakartalOpt requested a review from raju-opti July 29, 2025 15:01

esrakartalOpt and others added 2 commits July 30, 2025 10:07

Merge branch 'master' into esra/FSSDK-11458_eu_hosting

8e92fda

Fix test cases

dafb205

pvcraven requested a review from Copilot July 31, 2025 16:32

Copilot AI reviewed Jul 31, 2025

View reviewed changes

pvcraven reviewed Jul 31, 2025

View reviewed changes

jaeopt requested changes Jul 31, 2025

View reviewed changes

Mat001 requested changes Jul 31, 2025

View reviewed changes

esrakartalOpt added 2 commits July 31, 2025 15:26

Implement comments

c774dde

Fix lint issue

c645e73

Mat001 approved these changes Jul 31, 2025

View reviewed changes

jaeopt approved these changes Aug 1, 2025

View reviewed changes

esrakartalOpt merged commit 51eeb09 into master Aug 1, 2025
43 of 44 checks passed

esrakartalOpt deleted the esra/FSSDK-11458_eu_hosting branch August 1, 2025 14:57

	self.region = region or 'US'
	self.region = region or Region.US

		region = project_config.region.value if hasattr(project_config.region, 'value') else 'US'
		events_url = self.EVENTS_URLS.get(str(region), self.EVENTS_URLS['US'])

	{'do_you_know_me': 'test_value'}
	{'do_you_know_me': 'test_value',}

Conversation

esrakartalOpt commented Jul 2, 2025

Summary

Test plan

Issues

Uh oh!

raju-opti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

pvcraven left a comment

Choose a reason for hiding this comment

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

esrakartalOpt commented Jul 31, 2025

Uh oh!

Mat001 left a comment

Choose a reason for hiding this comment

Uh oh!

jaeopt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants