-
Notifications
You must be signed in to change notification settings - Fork 8
chore: Add FDv2 Polling Data Source #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ctional-data-source-updates
…rlamb/sdk-1584/fdv2-polling-data-source
…rlamb/sdk-1584/fdv2-polling-data-source
…rlamb/sdk-1584/fdv2-polling-data-source
0273b89 to
bfa1caa
Compare
pkgs/sdk/server/src/Internal/FDv2DataSources/FDv2PollingDataSource.cs
Outdated
Show resolved
Hide resolved
aa1998d to
0187fcf
Compare
0187fcf to
aee3377
Compare
|
bugbot review |
| _log.Debug("Get all flags returned 304: not modified"); | ||
| return null; | ||
| } | ||
| //We ensure the status code after checking for 304, because 304 isn't considered success |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this comment because it was blatantly wrong. A 304 is a success, and this code doesn't even run when there is a 304 because we return above. (I looked back through the history and it has been incorrect since addition 9 years ago.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no bugs!
pkgs/sdk/server/src/Internal/FDv2DataSources/FDv2PollingRequestor.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be FDv2Requestor? The only logic inside it that is specific to polling is the logs. I know off the top of my head iOS and Node have Requestor classes that are separable from the polling itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The payload it produces is not agnostic, so I am not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can always change it though, as it is internal.
Note
Implements FDv2 polling (requestor + data source) with event-array parsing, ETag/304 handling, status updates, env-id header extraction, and comprehensive tests.
Internal/FDv2DataSources/FDv2PollingDataSource.csimplementing periodic polling, processing protocol actions, applying transactional changesets, marking initialization, updatingDataSourceStatus, and handling JSON/HTTP errors.Internal/FDv2DataSources/FDv2PollingRequestor.csbuilding requests with selector query params, ETag caching, 304 handling, header capture (includingx-ld-envid), and deserializing pollingevents.IFDv2PollingRequestor.csdefiningFDv2PollingResponseand requestor interface.Internal/FDv2Payloads/FDv2Event.csaddsDeserializeEventsArrayand poll-event converter usage for parsingeventsarrays with strict validation.FDv2StreamingDataSource.cs(no functional change in this diff).Internal/DataSources/FeatureRequestor.cs.test/Internal/FDv2DataSources/FDv2PollingDataSourceTest.cscovering init, full/partial updates, deletes, 304 behavior, error/status transitions, selector propagation, and env-id extraction.test/Internal/FDv2Payloads/FDv2PayloadsTest.csforDeserializeEventsArrayand valid/invalid payload scenarios.Written by Cursor Bugbot for commit 69b18d1. This will update automatically on new commits. Configure here.