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.

Sign up for GitHub

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

Jump to bottom

Add avro_schema_nested field to schema_registry_decode / encode #90

Merged
merged 12 commits into from
Aug 20, 2024
Merged

Add avro_schema_nested field to schema_registry_decode / encode #90

merged 12 commits into from
Aug 20, 2024

Conversation

jem-davies
Copy link
Collaborator

No description provided.

@jem-davies
fix func resolveAvroReferences()
a7899a5 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies jem-davies linked an issue Aug 7, 2024 that may be closed by this pull request
schema_registry_decode fails to handle Avro schemas that nest other Avro schemas #76
Closed
@jem-davies
fix linter
13e56aa 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
remove panic()
ae0a3cd 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
add avro nested schema functionality
2fcad5f 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
make docs
2f5de5e 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies jem-davies changed the title fix func resolveAvroReferences() Add avro_schema_nested field to schema_registry_decode / encode Aug 11, 2024
@jem-davies jem-davies marked this pull request as ready for review August 11, 2024 15:25
jem-davies
jem-davies commented Aug 11, 2024
View reviewed changes
Copy link
Collaborator Author

@jem-davies jem-davies left a comment

Choose a reason for hiding this comment

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

reminder to update version numbers

internal/impl/confluent/processor_schema_registry_encode.go Outdated Show resolved Hide resolved
@jem-davies
add test for nested schemas
10985fe 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
alter version to 1.2.0
d40118b 
Signed-off-by: Jem Davies <jemsot@gmail.com>
gregfurman
gregfurman reviewed Aug 18, 2024
View reviewed changes
Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

Ran all the tests locally and seems to work. If possible, maybe some more context on the solution would be great. Otherwise, 1-2 small nits and a question re: that while loop when resolving nested references.

internal/impl/confluent/serde_avro_test.go Outdated Show resolved Hide resolved
internal/impl/confluent/serde_avro.go Show resolved Hide resolved
internal/impl/confluent/serde_avro.go Outdated Show resolved Hide resolved
@jem-davies
add line breaks to test data
1f73689 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
add maximum recursion limit to resolveAvroReferencesNested() + unit t…
a0deaaf 
…est, where there is a circular dep.

Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
fix lint
e191883 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
add mention of maximum depth on avro_nested_schemas field
29aae50 
Signed-off-by: Jem Davies <jemsot@gmail.com>
@jem-davies
make docs
c928001 
Signed-off-by: Jem Davies <jemsot@gmail.com>
gregfurman
gregfurman approved these changes Aug 20, 2024
View reviewed changes
Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM 🫡

@jem-davies jem-davies merged commit 63c8e61 into main Aug 20, 2024
3 checks passed
@jem-davies jem-davies deleted the 76-schema_registry_decode-fails-to-handle-avro-schemas-that-nest-other-avro-schemas branch August 20, 2024 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregfurman gregfurman gregfurman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

schema_registry_decode fails to handle Avro schemas that nest other Avro schemas
2 participants
@jem-davies @gregfurman