chore: Allow individual http settings in fdv2 #361
Conversation
| @inbox_full = Concurrent::AtomicBoolean.new(false) | ||
|
|
||
| event_sender = (test_properties || {})[:event_sender] || | ||
| Impl::EventSender.new(sdk_key, config, client || Impl::Util.new_http_client(config.events_uri, config)) |
There was a problem hiding this comment.
Changing this line has made the client parameter unused. Were we ever providing this and are we appropriately capturing that level of control that we previously had?
There was a problem hiding this comment.
The client was actually unused in the EventSender initializer and it just constructed its own client.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| def build(sdk_key, config) | ||
| http_opts = build_http_config | ||
| requester = @requester || HTTPPollingRequester.new(sdk_key, http_opts, config) | ||
| PollingDataSource.new(@poll_interval || DEFAULT_POLL_INTERVAL, requester, config.logger) |
There was a problem hiding this comment.
Builder defaults bypass user Config settings
Medium Severity
The data source builders now fall back to hardcoded DEFAULT_* constants instead of Config values when builder settings aren't explicitly set. Previously, PollingDataSourceBuilder.build used @config.poll_interval, but now uses @poll_interval || DEFAULT_POLL_INTERVAL. Similarly, StreamingDataSourceBuilder ignores config.initial_reconnect_delay and config.stream_uri, and build_http_config ignores config.base_uri, config.read_timeout, config.connect_timeout, and config.socket_factory. Users setting these values in Config while using DataSystem.default() or similar factory methods will have their settings silently ignored.
Additional Locations (2)
There was a problem hiding this comment.
The behavior is intentional by design—when data_system_config is not set, FDv1 is used and reads from Config; when using the new DataSystem API, configuration is done via the builder methods (like .poll_interval(), .base_uri()), not Config, as these are two separate and mutually exclusive configuration systems with synchronized defaults.
Tracked internally: SDK-1789
Note
Medium Risk
Touches core networking and FDv2 data acquisition construction paths (polling/streaming/event delivery) and changes builder interfaces; misconfiguration could cause incorrect endpoints/timeouts or break custom integrations relying on the old proc/initializer methods.
Overview
Refactors the FDv2 data system to use builder objects (with
#build(sdk_key, config)) instead of procs for initializers/synchronizers, updatingDataSystemfactories,DataSystemConfigdocs, and the FDv2 runtime to call.build.Introduces
HttpConfigOptionsplus aDataSourceBuilderCommonmixin so polling/streaming builders can independently configurebase_uri,socket_factory, and HTTP timeouts; polling and streaming requesters/data sources are updated to use these per-builder HTTP settings rather than pulling everything from globalConfig.Updates event sending and legacy polling request code to use the new HTTP config object and removes the injectable HTTP client from
EventSender. Contract tests and specs are adjusted to construct and pass the new builders (including a newTestDataV2builder and testMockBuilder) and to configure FDv1 fallback builder settings from the first available polling config.Written by Cursor Bugbot for commit c33b4d8. This will update automatically on new commits. Configure here.