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

Fix BytesSchema #1173

Merged
merged 1 commit into from Feb 20, 2024
Merged

Fix BytesSchema #1173

merged 1 commit into from Feb 20, 2024

Conversation

petermnhull
Copy link
Contributor

@petermnhull petermnhull commented Feb 10, 2024

This fixes #1175

Motivation

This fixes the BytesSchema by adding it into the switch-case in the NewBytes constructor, and adds a test showing that it can be used in a producer and consumer. Before the fix, the test fails due to the unrecognised schema in the constructor and the producer and consumer fail to encode the message.

The motivation is that BytesSchema appears to be intended as the default schema (e.g. see here for the Consumer) instead of defining no schema at all, but this code-path is never hit as no Schemas have the NONE type.

I first started looking at this when using a TableView with a topic without a defined or consistent Schema, and I have a work in progress branch for that which will rely on this.

Modifications

  • Add BytesSchema type into constructor for NewSchema.
  • Add test for BytesSchema usage.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

  • Added test showing NewBytes fails without the accompanying fix.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: yes, allows use of BytesSchema
  • The default values of configurations: no
  • The wire protocol: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable
  • If a feature is not applicable for documentation, explain why? small fix, equivalent functionality also has no documentation

@RobertIndie RobertIndie merged commit c2ca7e8 into apache:master Feb 20, 2024
8 checks passed
RobertIndie pushed a commit that referenced this pull request Feb 29, 2024
(cherry picked from commit c2ca7e8)
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.

Fix NewBytesSchema
2 participants