Fix DateTime field not being encoded as a numeric timestamp in JSON schema#356
Conversation
…chema ## Motivation Currently, the `DateTime` field is encoded as a string in the JSON schema, which is inconsistent with the type defined in Avro: ``` "CreatedDate": "2026-04-20T06:30:03.071518Z" ``` Instead, JSON schema payloads should align with the generated Avro logical timestamp schema, rather than representing `DateTime` fields as ISO-formatted strings. ## Modification * Add a `DateTime` JSON converter that outputs Unix epoch timestamps in milliseconds * Update JSON schema decoding to support both numeric timestamps and legacy ISO string formats
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78fd64456c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 995100f316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Updates Pulsar JSON schema serialization to encode DateTime fields as numeric Unix epoch timestamps (milliseconds) to better align JSON payloads with Avro logical timestamp schemas, while keeping backward-compatible decoding for older ISO-8601 string payloads.
Changes:
- Add a
System.Text.JsonJsonConverter<DateTime>that writesDateTimevalues astimestamp-millisnumbers. - Extend JSON decoding to accept both numeric timestamps and legacy ISO string timestamps.
- Adjust unit/integration tests to validate numeric encoding and millisecond precision expectations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Pulsar.Client/Schema/JsonSchema.fs |
Introduces the DateTime JSON converter and registers it in JsonSchema<'T> serializer options. |
tests/UnitTests/Internal/SchemaTests.fs |
Adds unit tests covering numeric DateTime JSON encoding/decoding and legacy string decode support. |
tests/IntegrationTests/Schema.fs |
Updates integration test expectations to match millisecond precision used by timestamp-millis payloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@RobertIndie I've done some edits, please review if you are fine with them |
Thanks! The changes look good to me. The CI should pass after rerunning. I just reran and verified it in my own PR: RobertIndie#12 |
Motivation
Currently, the
DateTimefield is encoded as a string in the JSON schema, which is inconsistent with the type defined in Avro:Instead, JSON schema payloads should align with the generated Avro logical timestamp schema, rather than representing
DateTimefields as ISO-formatted strings.Modification
DateTimeJSON converter that outputs Unix epoch timestamps in milliseconds