-
Notifications
You must be signed in to change notification settings - Fork 1.1k
DGS-20118: Upgrade log4j to log4j2 #3642
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
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request upgrades the code to use log4j2 while updating protobuf generated code accordingly. In addition, the changes update method calls and builder types in the WidgetProto and WidgetProto2 classes, and clarify doc comments in Decimal message types.
- Replaces calls to GeneratedMessageV3 with GeneratedMessage.
- Updates usage of SingleFieldBuilderV3 to SingleFieldBuilder.
- Enhances documentation by clarifying the semantics of the precision field in Decimal types.
Reviewed Changes
Copilot reviewed 8 out of 22 changed files in this pull request and generated no comments.
File | Description |
---|---|
schema-rules/src/test/java/io/confluent/kafka/schemaregistry/rules/WidgetProto2.java | Updates method calls for string writing and changes builder type; removes overrides for unknown field handling. |
schema-rules/src/test/java/io/confluent/kafka/schemaregistry/rules/WidgetProto.java | Switches method calls and builder types similarly, and removes unknown field override methods. |
protobuf-types/src/main/java/io/confluent/protobuf/type/DecimalOrBuilder.java | Improves the comment describing the precision field. |
protobuf-types/src/main/java/io/confluent/protobuf/type/Decimal.java | Updates the doc comments for precision in several locations. |
Files not reviewed (14)
- avro-serializer/src/test/resources/log4j.properties: Language not supported
- avro-serializer/src/test/resources/log4j2.xml: Language not supported
- benchmark/src/main/resources/log4j.properties: Language not supported
- benchmark/src/main/resources/log4j2.xml: Language not supported
- config/log4j.properties: Language not supported
- config/log4j2.xml: Language not supported
- core/src/main/resources/log4j.properties: Language not supported
- core/src/main/resources/log4j2.xml: Language not supported
- core/src/test/resources/log4j.properties: Language not supported
- core/src/test/resources/log4j2.xml: Language not supported
- protobuf-converter/src/test/resources/log4j.properties: Language not supported
- protobuf-converter/src/test/resources/log4j2.xml: Language not supported
- schema-rules/src/test/resources/log4j.properties: Language not supported
- schema-rules/src/test/resources/log4j2.xml: Language not supported
Comments suppressed due to low confidence (2)
schema-rules/src/test/java/io/confluent/kafka/schemaregistry/rules/WidgetProto2.java:3318
- The removal of the overridden setUnknownFields and mergeUnknownFields methods may affect unknown fields handling. Please verify that this change does not introduce regressions in message parsing behavior.
@java.lang.Override
schema-rules/src/test/java/io/confluent/kafka/schemaregistry/rules/WidgetProto.java:3015
- The removal of the overridden setUnknownFields and mergeUnknownFields methods could impact the handling of unknown fields. Confirm that this change is intentional and does not compromise message consistency.
@java.lang.Override
e8b8b70
to
f8ddd85
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a minor comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @GunalKupta , LGTM
What
Checklist
References
JIRA: DGS-20118
Test & Review
Ran
bin/schema-registry-start config/schema-registry.properties
locally and verified that the logs look identical when run in this branch and master branchOpen questions / Follow-ups