Skip to content

Conversation

timothycoleman
Copy link
Contributor

@timothycoleman timothycoleman commented Jul 15, 2025

Log record properties that are already captured elsewhere in the log record need not be stored as properties. They are reconstituted on read.

i.e.

  • EventType
  • ContentType if it is fully described by the IsJson flag (application/json or application/octet-stream)

@timothycoleman timothycoleman changed the title Do not populate log record properties on streams v1 API [KDB-803] Do not populate log record properties on streams v1 API Jul 15, 2025
Copy link
Contributor

github-actions bot commented Jul 15, 2025

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

@timothycoleman timothycoleman force-pushed the timothycoleman/properties-metadata-compatibility branch 2 times, most recently from 10ca4cf to 6ad23bb Compare July 15, 2025 16:22
Base automatically changed from sergio/streams-v2-json-metadata to master July 16, 2025 12:17
@timothycoleman timothycoleman force-pushed the timothycoleman/properties-metadata-compatibility branch 4 times, most recently from f29b150 to a5ae98d Compare July 17, 2025 06:10
@timothycoleman timothycoleman changed the base branch from master to timothycoleman/do-not-populate-both-metadata-and-properties July 17, 2025 06:10
@timothycoleman timothycoleman force-pushed the timothycoleman/properties-metadata-compatibility branch 2 times, most recently from e38ef42 to 4730ac8 Compare July 17, 2025 09:13
@timothycoleman timothycoleman changed the title [KDB-803] Do not populate log record properties on streams v1 API [KDB-803] Only store non-redundant log record properties Jul 17, 2025
Base automatically changed from timothycoleman/do-not-populate-both-metadata-and-properties to master July 21, 2025 13:19
- Do not store EventType as a property, it is stored elsewhere in the log record
- Do not store DataFormat as a property when it is fully described by IsJson (i.e. when Json or Bytes)
- Reject DataFormats that are not one of the 4 allowed.
@timothycoleman timothycoleman force-pushed the timothycoleman/properties-metadata-compatibility branch from 4730ac8 to c511a36 Compare July 21, 2025 13:32
@timothycoleman timothycoleman marked this pull request as ready for review July 21, 2025 13:32
@timothycoleman timothycoleman requested a review from a team as a code owner July 21, 2025 13:32
Copy link
Contributor

@RagingKore RagingKore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

public static class DataFormats {
// can be others (protobuf, avro, ...) but json is the only one the server cares about
public const string Json = "json";
public const string Bytes = "bytes";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a future PR we should ensure we use the typed proto enum for the Data Formats.

@timothycoleman timothycoleman merged commit 1060714 into master Jul 21, 2025
11 of 13 checks passed
@timothycoleman timothycoleman deleted the timothycoleman/properties-metadata-compatibility branch July 21, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants