Skip to content
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

feat(sdk)!: Do not set metadata defaults in Stream constructor #2851

Merged
merged 19 commits into from
Nov 5, 2024

Conversation

teogeb
Copy link
Contributor

@teogeb teogeb commented Oct 30, 2024

This is a breaking change as the value returned by getMetadata() is different.

The Stream constructor no longer sets default values for any metadata fields. Before this PR the constructor initialized values for partitions and fields values. As metadata is now untyped (#2845), it makes sense to drop these defaults.

Also the internal Streamr#parseMetadata() no longer sets default value for the partitions field .

Implications

Stream#getMetadata() now returns metadata exactly as it stored in the stream registry.

  • But we can easily query the partition count with the Stream#getPartitionCount() method. It defaults to the DEFAULT_PARTITION_COUNT if there is no value available in metadata.
  • And very typically the partition count has been explicitly stored to stream registry metadata, so it is available also when queried via Stream#getMetadata().partitions.

@github-actions github-actions bot added the sdk label Oct 30, 2024
@github-actions github-actions bot added the docs label Oct 30, 2024
@teogeb teogeb marked this pull request as ready for review October 30, 2024 22:51
@teogeb teogeb requested review from juslesan and jtakalai October 30, 2024 23:19
Base automatically changed from stream-uptyped-metadata to main November 5, 2024 13:25
@teogeb teogeb merged commit 78f9965 into main Nov 5, 2024
24 checks passed
@teogeb teogeb deleted the do-not-set-defaults-for-metadata-in-Stream-constructor branch November 5, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants