-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[Java][jersey2] Fix logging for jersey2 (Java6) #7348
[Java][jersey2] Fix logging for jersey2 (Java6) #7348
Conversation
@wing328 |
@SergeyLyakhov We'll sync the change in master to those branches (2.4.0, 3.0.0) later |
clientConfig.register(new LoggingFeature(java.util.logging.Logger.getLogger(LoggingFeature.DEFAULT_LOGGER_NAME), java.util.logging.Level.INFO, LoggingFeature.Verbosity.PAYLOAD_ANY, 1024*50 /* Log payloads up to 50K */)); | ||
clientConfig.property(LoggingFeature.LOGGING_FEATURE_VERBOSITY, LoggingFeature.Verbosity.PAYLOAD_ANY); | ||
// Set logger to ALL | ||
java.util.logging.Logger.getLogger(LoggingFeature.DEFAULT_LOGGER_NAME).setLevel(java.util.logging.Level.ALL); | ||
{{/supportJava6}} | ||
{{#supportJava6}} | ||
clientConfig.register(new LoggingFilter(java.util.logging.Logger.getLogger(LoggingFilter.class.getName()), true)); |
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.
Please align this line properly by removing 2 spaces.
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.
Fixed.
@SergeyLyakhov thanks again for the PR. When I tried to test
Ref: https://travis-ci.org/swagger-api/swagger-codegen/builds/327592947 Did you get similar result when running |
@wing328 Will update PR tomorrow if you agree with proposed changes. |
@SergeyLyakhov I forgot to mention that I did update the pom to use 1.6: https://github.com/swagger-api/swagger-codegen/blob/SergeyLyakhov-bugfix/jersey2-java6-logging/samples/client/petstore/java/jersey2-java6/pom.xml#L141-L142 |
@SergeyLyakhov I've merged your fix into master. If you later can find a way to make it work with |
PR checklist
[+] Read the contribution guidelines.
[+] Ran the shell script under
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.[-] Filed the PR against the correct branch:
3.0.0
branch for changes related to OpenAPI spec 3.0. Default:master
.Committed it into master branch, because expect this fix in 2.4.0. May create PR for 3.0 as well.
[+] Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
@bbdouglas @JFCote @sreeshas @jfiala @lukoyanov @cbornet @wing328
Description of the PR
Jersey 2.6 (used with Java6) doesn't support LoggingFeature (fix SHA-1: 2d19776
This change reverts to the old logging logic for Jersey2 + java6.