-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
Update avro to 1.11.3 #511
Conversation
Namespace for nested classes no longer ends with '$'. This is how avro library generates schema since version 1.9. See: AVRO-2143 Please note that resolution of nested classes without '$' was implemented long ago in c570549. Fixes FasterXML#167
avro/src/main/java/tools/jackson/dataformat/avro/schema/AvroSchemaHelper.java
Outdated
Show resolved
Hide resolved
@rafalh Thanks - I ran the unit tests with your changes and the tests all pass. @cowtowncoder the changes look small enough and if users want to use the older Avro releases supported by jackson-dataformat-avro 2.17 - they can just not upgrade. If you run the tests with the avro version set back 1.8.2 but with the other code changes in this PR - you get All tests pass if you use avro 1.9.2. |
…levels Avro before 1.11 was generating schemas with '$' in namespace if class had multiple nesting levels. To fix compatibility with avro 1.11+ make sure all dollar characters are replaced by dots. Related: AVRO-2757
Ok so this would only be for Jackson 3.0 ( Depending on compatibility constraints -- I am not 100% sure I understand constraints wrt Avro schema handling. The important thing would be ability for endpoints where one uses older Jackson Avro module and another newer would still work correctly wrt wire format. |
I wasn't sure what the target branch of the PR should be. I assumed that if the change is evaluated to be safe for back-porting, it will get cherry-picked to other branches, but if that's not your workflow I can switch to
If I understand correctly over-the-wire compatibility should be preserved. Also working with external schema which uses namespace with dollar signs should work fine, because I didn't remove handling for it: https://github.com/FasterXML/jackson-dataformats-binary/pull/511/files#diff-6199d0c495f8d8ed14a8059be2c4607ca408f8a4733390bb78350fec5d8010a9R341 The only problem is when someone generates schema from POJO with nested classes in new Jackson and then tries to use that schema with old Jackson. I don't think it's a popular use-case. Schema producer can always stick to older version if needed. If this is acceptable I am all for merging it to 2.18. Alternative solution would be to add a new feature to |
@rafalh the package names in master branch were changed from the values used in the 2.18 branch. Could you do a new PR that implements this set of changes but target the 2.18 branch instead? Leave this PR open too because it is useful for merging to the master branch. |
Yes, like @pjfanning mentioned, merging across 2.x and master won't work with cherry-picking (due to multiple refactorings, renamings mostly) and so almost always starting with 2.x then resolving renames, package imports when merging to Thank you for creating 2.18 PR -- I hope to get that merged soon. |
Was able to merge forward #512, closing this one. |
Update Apache avro library to 1.11.3.
To make it compatible namespace generation for nested classes had to be changed.
Now dots are used instead of dollar signs as indicator of nesting.
AFAIK dollar sign is not an allowed character in Avro namespace according to the specification and this is why this change was implemented in avro library.
Note that avro library generates namespaces not ending with dollar character since version 1.9, but it actually removes all dollar signs in classes with multiple levels of nesting since 1.11.
See: AVRO-2143 and AVRO-2757
Fixes #167