[cli] Don't log to STDOUT if debug flags are set #474

devplayer0 · 2018-07-05T11:06:34Z

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: master, 3.1.x, 4.0.x. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

It makes sense that error messages should be written to STDERR and
all others should be written to STDOUT (as shown in #207). However, it
would be convenient to parse the debugging output when the relevant
flags are set.

This change will disable logging to STDOUT and redirect all log messages
to STDERR when any of the debug flags are set. (Resolves #473)

wing328 · 2018-07-18T10:34:32Z

@devplayer0 thanks for the PR. Shall we let the users control the behavior using 1>&2 instead so that the users have the option to decide where the log messages go?

devplayer0 · 2018-07-18T10:43:36Z

@wing328 Normally that's what I'd do, but the problem is that there's no way to filter out the log output from the JSON when flags like -DdebugOperations are set.

With this change, I can do java -DdebugModels -jar openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i cellar.json -o /tmp/cellar > generate.json and the contents generate.json will be parsed correctly by a JSON viewer, while the log messages will be printed to STDERR.

jimschubert · 2018-07-30T01:12:09Z

I think this is a good solution, but I think it would make more sense to put this behind an opt-in switch on the command line (something like --debug-to-stdout or --debug-print-only).

I use the debug options often, and while I understand the use case… I would find it annoying to not see the other STDOUT messages when using the debug options.

It makes sense that error messages should be written to STDERR and all others should be written to STDOUT (as shown in OpenAPITools#207). However, it would be convenient to parse the debugging output when the relevant flags are set. This change will disable logging to STDOUT and redirect all log messages to STDERR when any of the debug flags are set. (Resolves OpenAPITools#473)

Instead of automatically redirecting log messages to STDERR upon detection of the debug options, a new command line option (`--log-to-stderr`) will trigger this behavour, allowing the user to decide whether or not this is something they want.

devplayer0 · 2018-08-17T12:08:26Z

@wing328 @jimschubert I have changed the behaviour so that log messages will only be redirected to STDERR if the --log-to-stderr flag is set. (Instead of just checking for the presence of the debug options)

For example, see the following commands:

java -DdebugModels -jar openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i cellar.json -o /tmp/cellar > generate.log
java -DdebugModels -jar openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar generate --log-to-stderr -g java -i cellar.json -o /tmp/cellar > generate.json

The contents of generate.log would be all (non-error) log messages alongside the JSON output of the debug options as normal. generate.json would contain only the debug options output. (with all log messages being written to STDERR)

jimschubert · 2018-08-20T01:32:04Z

I've removed the breaking change label, as this is now a new option with no breaking change.

jimschubert

Looks good. No longer a breaking change, which is awesome.

I'll merge and update the README separately to list the new option.

jimschubert · 2018-08-20T01:32:43Z

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java

@@ -199,8 +202,23 @@
            description = "Skips the default behavior of validating an input specification.")
    private Boolean skipValidateSpec;

+    @Option(name = {"--log-to-stderr"},


I think this option is much clearer, and has the benefit of not being a breaking change.

jimschubert · 2018-08-20T01:39:20Z

Updated README via a3e5edc

* master: Fix problems in typescript jquery generator (#801) [PHP] Add gitignore to AbstractPhpCodegen (#765) Improve documentation for usage of a generator in an other jar (#817) Improve Symfony 4.1 compatibility (#830) 📝 Updating 'help generate' switches in README [cli] Don't log to STDOUT if debug flags are set (#474) [gradle-plugin] README notes on multiple specs (#847) Added server variable support (#816) fix erlang optiona/required parameters (#829) [Java][Rest-assured] Fix generated javadoc and "swagger-annotations" improvement (#831)

wing328 · 2019-01-02T07:39:38Z

@devplayer0 thanks for the contribution, which has been included in the 4.0.0-beta release: https://twitter.com/oas_generator/status/1079727020374806529.

Happy New Year and looking forward to more collaboration and contributions in 2019!

It makes sense that error messages should be written to STDERR and all others should be written to STDOUT (as shown in OpenAPITools#207). However, it would be convenient to parse the debugging output when the relevant flags are set. This change will disable logging to STDOUT and redirect all log messages to STDERR when any of the debug flags are set. (Resolves OpenAPITools#473)

devplayer0 mentioned this pull request Jul 5, 2018

[cli] Generator logs to STDOUT when debug options are set #473

Closed

devplayer0 force-pushed the debug-log-stderr branch from f2d0f4b to 331c025 Compare July 12, 2018 09:15

devplayer0 force-pushed the debug-log-stderr branch from 331c025 to bd2bfe5 Compare July 18, 2018 10:43

wing328 added the OpenAPI Generator CLI label Jul 23, 2018

wing328 modified the milestones: 3.2.0, 4.0.0 Jul 23, 2018

wing328 added the Breaking change (without fallback) label Jul 23, 2018

devplayer0 added 2 commits August 17, 2018 13:00

Don't automatically change the log config

5a543bc

Instead of automatically redirecting log messages to STDERR upon detection of the debug options, a new command line option (`--log-to-stderr`) will trigger this behavour, allowing the user to decide whether or not this is something they want.

devplayer0 force-pushed the debug-log-stderr branch from bd2bfe5 to 5a543bc Compare August 17, 2018 12:02

jimschubert self-assigned this Aug 20, 2018

jimschubert removed the Breaking change (without fallback) label Aug 20, 2018

jimschubert approved these changes Aug 20, 2018

View reviewed changes

jimschubert merged commit bd7c9e3 into OpenAPITools:master Aug 20, 2018

devplayer0 deleted the debug-log-stderr branch August 20, 2018 13:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] Don't log to STDOUT if debug flags are set #474

[cli] Don't log to STDOUT if debug flags are set #474

devplayer0 commented Jul 5, 2018

wing328 commented Jul 18, 2018

devplayer0 commented Jul 18, 2018

jimschubert commented Jul 30, 2018

devplayer0 commented Aug 17, 2018

jimschubert commented Aug 20, 2018

jimschubert left a comment

jimschubert Aug 20, 2018

jimschubert commented Aug 20, 2018

wing328 commented Jan 2, 2019

[cli] Don't log to STDOUT if debug flags are set #474

[cli] Don't log to STDOUT if debug flags are set #474

Conversation

devplayer0 commented Jul 5, 2018

PR checklist

Description of the PR

wing328 commented Jul 18, 2018

devplayer0 commented Jul 18, 2018

jimschubert commented Jul 30, 2018

devplayer0 commented Aug 17, 2018

jimschubert commented Aug 20, 2018

jimschubert left a comment

Choose a reason for hiding this comment

jimschubert Aug 20, 2018

Choose a reason for hiding this comment

jimschubert commented Aug 20, 2018

wing328 commented Jan 2, 2019