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: add ids to channel and schema types #1245

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alxhill
Copy link

@alxhill alxhill commented Sep 30, 2024

Changelog

Add id fields to the Schema and Channel structs.

Docs

None

Description

The Channel ID is needed to be able to distinguish two channels with the same topic name, which are not guaranteed to be unique. I've also added the ID to the Schema type for consistency but this is not strictly necessary for us/our use case.

One challenge here is that the Channel and Schema types are used for both read and write workflows, but for write workflows it would be ignored (e.g add_channel generates a new channel id). In this PR I ignored the additional field, but would be happy to refactor another way. Some options I came up with:

  1. make the field Option<u16> and set it on read. Simple enough, but makes the contract of when it will/won't be set (or how it'll behave if the field is set on write) unclear.
  2. create a new MessageStream that returns message header with the message (or some other new combined type)
  3. Split the Channel struct into readable/writeable versions. Seems like a fairly invasive change but may be helpful in the long-run?

@alxhill alxhill marked this pull request as draft September 30, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant