8000 Resolve whole references tree by kleewho · Pull Request #250 · deviceinsight/kafkactl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Resolve whole references tree #250

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
Merged
merged 3 commits into from
May 13, 2025
Merged

Resolve whole references tree #250

merged 3 commits into from
May 13, 2025

Conversation

kleewho
Copy link
Contributor
@kleewho kleewho commented Apr 5, 2025 •
edited
Loading

Instead of just 1 level of references it's going
to download all levels.
Also use Name instead of Subject as a reference filename

I don't know what I was thinking with this resolve not going deeper initially.

And about the Subject -> Name change. Here's the part from the Confluent documentation:

A schema reference consists of the following:
* A name for the reference. (For Avro, the reference name is the fully qualified schema name, for JSON Schema it is a URL, and for Protobuf, it is the name of another Protobuf file.)
* A subject, representing the subject under which the referenced schema is registered.
* A version, representing the exact version of the schema under the registered subject.

I made a mistake and used just wrong thing

Description

Please include a summary of the change and which issue is fixed.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Documentation

  • the change is mentioned in the ## [Unreleased] section of CHANGELOG.md
  • the configuration yaml was changed and the example config in README.adoc was updated
  • a usage example was added to README.adoc
  • tests for the changes have been implemented (see: Testing your changes)

8000 @kleewho
Resolve whole references tree
4160d7b 
Instead of just 1 level of references it's going
to download all levels.
Also use Name instead of Subject as a reference filename
d-rk
d-rk reviewed Apr 15, 2025
View reviewed changes
Copy link
Collaborator
@d-rk d-rk left a comment

Choose a reason for hiding this comment

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

currently there is no integration test that checks that the proto deserialization with dependencies works as intended.
It would be good if you could add one 😃

@kleewho
Copy link
Contributor Author
kleewho commented Apr 24, 2025

Sure. I'll add a test

@kleewho
Copy link
Contributor Author
kleewho commented May 9, 2025

Test added. Thanks to this I noticed some issues and fixed them

@d-rk
Copy link
Collaborator
d-rk commented May 9, 2025

👍 I will have a look next week

@d-rk d-rk merged commit d7ff6b0 into deviceinsight:main May 13, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d-rk d-rk d-rk left review comments

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

Successfully merging this pull request may close these issues.
2 participants
@kleewho @d-rk
0