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

schema_registry/test: Support JSON custom attributes on fields #7330

Merged
merged 2 commits into from
Nov 17, 2022
Merged

schema_registry/test: Support JSON custom attributes on fields #7330

merged 2 commits into from
Nov 17, 2022

Conversation

BenPope
Copy link
Member

@BenPope BenPope commented Nov 16, 2022

Cover letter

Test that #7274 is mitigated.

Signed-off-by: Ben Pope ben@redpanda.com

Backport Required

  • not a bug fix
  • issue does not exist in previous branches
  • papercut/not impactful enough to backport
  • v22.3.x
  • v22.2.x
  • v22.1.x

UX changes

  • none

Release notes

Improvements

  • schema_registry: AVRO: Support custom attributes on fields (fix a regression since v22.2.x)

@BenPope
Update Avro
3f95ed8 
Signed-off-by: Ben Pope <ben@redpanda.com>
@BenPope
schema_registry: Support JSON custom attributes on fields
eb18a6b 
Signed-off-by: Ben Pope <ben@redpanda.com>
@BenPope BenPope requested a review from a team as a code owner November 16, 2022 20:44
@BenPope BenPope requested review from rishabh96b and removed request for a team November 16, 2022 20:44
@BenPope BenPope removed the request for review from rishabh96b November 16, 2022 20:53
NyaliaLui
NyaliaLui approved these changes Nov 16, 2022
View reviewed changes
src/v/pandaproxy/schema_registry/test/compatibility_avro.cc
pps::schema_type::avro})
.value();
pps::canonical_schema_definition expected{
R"({"type":"record","name":"foo","fields":[{"name":"bar","type":"float","extra_attr":true}]})",
Copy link
Contributor

Choose a reason for hiding this comment

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

So we drop the custom attribute on the schema (ignored_attr) but not the one on the field (extra_attr)?

This isn't a blocker but more of a Q as to why dropping one and not the other is OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just going by what's written here: apache/avro#1736

@BenPope
Copy link
Member Author

BenPope commented Nov 16, 2022 •
edited

CI Failures:
Too many to look at - none of them to do with the schema registry:

Module: rptest.tests.upgrade_test
Class:  UpgradeFromPriorFeatureVersionTest
Method: test_basic_upgrade

Module: rptest.tests.partition_movement_test
Class:  PartitionMovementTest

Module: rptest.tests.partition_movement_test
Class:  SIPartitionMovementTest

Module: rptest.tests.upgrade_test

Module: rptest.tests.license_upgrade_test
Class:  UpgradeMigratingLicenseVersion
Method: test_license_upgrade

They all seemed to fail at about 23:05:30

Debug seemed to fail similarly.

@BenPope BenPope merged commit c7992b7 into redpanda-data:dev Nov 17, 2022
@BenPope
Copy link
Member Author

BenPope commented Nov 17, 2022

/backport v22.3.x

@vbotbuildovich vbotbuildovich mentioned this pull request Nov 17, 2022
Merged
@chenrui333 chenrui333 mentioned this pull request Dec 24, 2022
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotnwat dotnwat dotnwat approved these changes

@NyaliaLui NyaliaLui NyaliaLui approved these changes

Assignees
No one assigned
Labels
area/build area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@BenPope @dotnwat @NyaliaLui