Backport avro canonicalizer #4278
Conversation
|
Thank you for creating a pull request! Pinging @jsenko to respond or triage. |
| @@ -29,26 +30,27 @@ | |||
| @ApplicationScoped | |||
| public class KafkaSqlUpgrader { | |||
There was a problem hiding this comment.
Realizing that this probably contributes to slow startup times for kafkasql, eh? Perhaps for 3.0 we should consider excluding hashes and similar from the Kafka messages and generate them during message consumption. I'm not sure if that's feasible or not, but it would mean we wouldn't need to run these upgraders.
There was a problem hiding this comment.
Note: this has nothing to do with your PR, just a thought I had while reviewing. :)
|
Seems the kafkasql upgrade test timed out. |
carlesarnal
force-pushed
the
backport-avro-canonicalizer
branch
from
9cad26b to
aa202c3
Compare
Back to this. Yes, there was an actual bug in the upgrader causing it. This is mostly done, now I have to create an isolated test for the AvroCanonicalizerUpgrader and it will be good to go. |
carlesarnal
force-pushed
the
backport-avro-canonicalizer
branch
2 times, most recently
from
c9fdefe to
42d1816
Compare
carlesarnal
force-pushed
the
backport-avro-canonicalizer
branch
4 times, most recently
from
03c6a3f to
b3ceb8d
Compare
carlesarnal
force-pushed
the
backport-avro-canonicalizer
branch
from
b3ceb8d to
11e2587
Compare
carlesarnal
force-pushed
the
backport-avro-canonicalizer
branch
from
11e2587 to
68f3726
Compare
This is backporting the original PR #300 to 2.5.x. During the testing, I discovered a bug, the new canonicalizer was not properly handling Avro schemas with nested records. I have addressed this problem here.
Creating in draft since I still have to write a specific test for it (The existing ones also validate this scenario, but I want a specific one just for it).
cc @AndreaScarselli