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

[v24.2.x] CORE-6860 Schema Registry: add detailed verbose compatibility checks #23110

Conversation

pgellert
Copy link
Contributor

@pgellert pgellert commented Aug 28, 2024

Backport of PR #22958

Fixes #23107
Fixes https://redpandadata.atlassian.net/browse/CORE-7073

Conflicts:

  • False conflict around proto_compatibility_result check_compatible(std::filesystem::path) in 4520d58. The commits should be identical to the one being cherry-picked.

Common test code for working with detailed compatibility info about
protobuf and avro schemas.

(cherry picked from commit 11a87c3)
@pgellert pgellert added this to the v24.2.x-next milestone Aug 28, 2024
@pgellert pgellert added the kind/backport PRs targeting a stable branch label Aug 28, 2024
@pgellert pgellert self-assigned this Aug 28, 2024
@pgellert pgellert marked this pull request as ready for review August 28, 2024 14:57
@pgellert pgellert requested review from oleiman and BenPope August 28, 2024 17:44
Comment on lines 483 to 484
// There must be a compatible reader message for every writer
// message
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't appear in the upstream diff. no clue why 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must have formatted the file while resolving the merge conflict which shifted the last word to a new line. I've fixed this now to match the original PR.

Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

lgtm modulo that weird code comment

Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

LGTM, aside from the comment

oleiman and others added 2 commits September 2, 2024 17:45
When an incompatible schema is posted, include the verbose compatibility
messages in the error message.

This helps users understand the reason for the incompatibility and it
matches the reference implementation.

Includes a minor fix to the way the compatibility level message is
formatted.

(cherry picked from commit cda768b)
@pgellert pgellert force-pushed the vbotbuildovich/backport-22958-v24.2.x-625 branch from 0a97039 to 51ff964 Compare September 2, 2024 16:46
@pgellert
Copy link
Contributor Author

pgellert commented Sep 2, 2024

Force-push: fix comment formatting

@pgellert pgellert requested review from BenPope and oleiman September 2, 2024 16:49
@pgellert pgellert merged commit c0bfc7d into redpanda-data:v24.2.x Sep 3, 2024
20 checks passed
@BenPope BenPope modified the milestones: v24.2.x-next, v24.2.4 Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants