fix: Convert datetime attributes to ISO-8601 strings - #61
Draft
kinyoklion wants to merge 1 commit into
Draft
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor
|
@cursor review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Evaluation context attributes are now converted to values the LaunchDarkly context model can represent, instead of being passed through as-is.
datetimeattribute previously reached the LaunchDarkly context unchanged, so the context could not be serialized:json.dumpson it raisesTypeError: Object of type datetime is not JSON serializable, which breaks analytics event delivery for that context.datetimevalues now become their ISO-8601 representation in UTC, which is the format the LaunchDarkly date targeting operators accept. A naivedatetimeis treated as UTC.datetimeis converted too.Noneand logs an error, rather than producing an unusable context.This aligns the Python provider with the Java, .NET, and JavaScript providers, which already convert their SDK's date values, and with the
date or instantrow of the OpenFeature provider behavior spec's value conversion table.Found during the weekly OpenFeature provider audit.
Implementation details
Requirements
Related issues
None.
Describe the solution you've provided
_to_ld_valuein the context converter handles the OpenFeature value types: scalars pass through, adatetimeis converted withastimezone(timezone.utc).isoformat()(with the+00:00offset written asZ), mappings and sequences are converted item by item, and anything else becomesNonewith an error log. Custom attributes are set through it;name,anonymous, andprivateAttributeskeep their existing type-specific handling.datetimeis one of the types the OpenFeature Python SDK'sEvaluationContextAttributealias allows, so an application following the OpenFeature specification can supply one.Describe alternatives you've considered
Rejecting a
datetimeattribute and logging an error: that loses targeting information the application intended to send, and date targeting operators exist precisely for this case.Treating a naive
datetimeas local time: the local zone of a server is incidental, so the same context would convert differently depending on where it ran.Additional context
Testing:
make lint(mypy) andmake test(86 tests). The new tests cover an awaredatetime, a non-UTC offset, a naivedatetime, adatetimenested inside a dictionary and list, JSON serializability of the resulting context, and an unconvertible value.The README's OpenFeature specific considerations section documents the conversion.
Link to Devin session: https://app.devin.ai/sessions/fe1eb757fe694ef79f3d09f6307d4b47
Open in Devin Desktop: https://app.devin.ai/desktop/session/fe1eb757fe694ef79f3d09f6307d4b47?variant=devin
Requested by: @kinyoklion