[Kayvan-Zahiri]

Problem

EventAPIClient.publish_events passes event_request_timeout_ms directly to requests.Session.request, whose timeout parameter is in seconds. The default 10000 ms is therefore sent as 10000 seconds (2 hours 46 minutes 40 seconds), rather than 10 seconds. Custom subsecond values are likewise off by a factor of 1000.

Change

Convert the configured milliseconds to seconds at the HTTP request boundary, matching the existing config client's conversion. Retry behavior and the configured option remain unchanged.

Validation

• Four regression tests exercise publish_events with the default, 1 ms, 250 ms, and 1250 ms settings. responses verifies the actual Requests timeout argument and returns a successful event-batch response only for the expected seconds value.
• Before: 4 new cases fail; 6 existing event-client controls pass.
• After: full SDK suite 152 passed, 1 skipped, with benchmark tests excluded by the repository's normal pytest configuration.
• Full Ruff and Black checks pass; full MyPy passes for 69 source files; git diff --check passes.
• Tests use mocked HTTP and a test SDK key; no hosted DevCycle requests or paid resources were used. This verifies the HTTP timeout argument, not real elapsed-time behavior against a stalled server.

AI assistance: OpenAI Codex assisted with reproduction, patch, tests, and this description.

[jonathannorris]

Thanks for the contribution @Kayvan-Zahiri! Good catch on the ms vs seconds mismatch, the fix itself looks good and matches what config_client.py already does.

I think it probably makes sense to simplify the tests a bit though:

1. Assert on the timeout directly instead of through request_kwargs_matcher. With the matcher, a wrong value surfaces as a ConnectionError from responses that gets wrapped in APIClientError: Retries exceeded, so you have to dig through the logs to see why it failed. responses already records the request kwargs, so asserting on responses.calls[0].request.req_kwargs["timeout"] fails with 10000 != 10.0 instead.
2. Move them into EventAPIClientTest in test/api/test_event_client.py, which already sets up the client, batch URL, and batch in setUp, rather than adding a new file.
3. Two cases are enough: the default and one custom value. The four cases are all exercising the same division.

Something like:

    @responses.activate
    def test_publish_events_timeout_in_seconds(self):
        responses.add(responses.POST, self.test_batch_url, json={"message": "ok"})
        self.test_client.publish_events(self.test_batch)
        self.assertEqual(responses.calls[0].request.req_kwargs["timeout"], 10.0)

    @responses.activate
    def test_publish_events_custom_timeout_in_seconds(self):
        client = EventAPIClient(
            self.sdk_key,
            DevCycleLocalOptions(
                events_api_uri="http://localhost:8080", event_request_timeout_ms=250
            ),
        )
        responses.add(responses.POST, self.test_batch_url, json={"message": "ok"})
        client.publish_events(self.test_batch)
        self.assertEqual(responses.calls[0].request.req_kwargs["timeout"], 0.25)

and test/api/test_event_timeout.py can be dropped.

If it's easier, I'm happy to make these changes myself in a separate branch, just let me know.

[Kayvan-Zahiri]

Thanks, done in 2ec10c6. The two tests now live in EventAPIClientTest and assert on req_kwargs["timeout"], and test_event_timeout.py is gone. With the fix reverted they fail with 10000 != 10.0 and 250 != 0.25.