Skip to content
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

Combining -json and -logFile yields no logFile #2582

Closed
esauser opened this issue Nov 29, 2019 · 10 comments
Closed

Combining -json and -logFile yields no logFile #2582

esauser opened this issue Nov 29, 2019 · 10 comments

Comments

@esauser
Copy link

@esauser esauser commented Nov 29, 2019

Which version and edition of Flyway are you using?

6.1.0

If this is not the latest version, can you reproduce the issue with the latest one as well?

(Many bugs are fixed in newer releases and upgrading will often resolve the issue)

Which client are you using? (Command-line, Java API, Maven plugin, Gradle plugin)

Command-line

Which database are you using (type & version)?

MSSQL 2017 CU17 in Docker

Which operating system are you using?

MacOS

What did you do?

(Please include the content causing the issue, any relevant configuration settings, the SQL statement that failed (if relevant) and the command you ran.)
flyway info -json -logFile="test"

What did you expect to see?

A test file created with JSON formatting of the output

What did you see instead?

Nothing. -json and -logFile function independently, but they do not function together. Instead, JSON formatting is printed to standard out, but the log file is not created.

@alextercete alextercete added this to the Flyway 6.1.1 milestone Dec 2, 2019
@MikielAgutu
Copy link
Contributor

@MikielAgutu MikielAgutu commented Dec 3, 2019

Thanks for pointing this out. We agree it's inconsistent that -logFile produces no output when -json is set.

We're thinking that the normal logs (INFO, DEBUG, etc) should be written to the -logFile, whereas JSON should only be placed on the console output. Do you have a specific use case for JSON in the log file?

@esauser
Copy link
Author

@esauser esauser commented Dec 3, 2019

Yes, I'm currently working around this limitation by piping the output to a file. I'm consuming the output in a CI/CD deployment pipeline where one step generates the output and another consumes it. Thus, I need to persist it to a file instead of piping straight through to the next command.

@MikielAgutu
Copy link
Contributor

@MikielAgutu MikielAgutu commented Dec 5, 2019

After more consideration we think it makes sense for the 'normal' logs to go to the log file, and the JSON only to the console. Piping the output to a file seems like a reasonable compromise.

@esauser
Copy link
Author

@esauser esauser commented Dec 5, 2019

@MikielAgutu I don't think anyone will expect that behavior. Every CLI I've ever used sends the same thing to output and log file when specified. I recommend the same is done here. Additionally, not having to pipe make CI/CD integration a lot easier since it is just an arg instead of a completely separate command.

@alextercete
Copy link
Contributor

@alextercete alextercete commented Dec 5, 2019

@esauser I agree that the output of the CLI should be sent to the log file when the -logFile option is specified. However, I couldn't find any examples of CLIs that output JSON to the log file (when some option like -json or --output json is specified). Can you think of a specific example?

Additionally, not having to pipe make CI/CD integration a lot easier since it is just an arg instead of a completely separate command.

I didn't quite understand this point, would you mind giving an example?

@esauser
Copy link
Author

@esauser esauser commented Dec 5, 2019

@alextercete I do not have an example in mind. But I've never seen one that didn't send the same to the log file that it sent to output.

An example in CI/CD is with Concourse. The run configuration for Concourse tasks takes a path and args. Ideally, we'd call Flyway like this

path: /flyway/flyway
args: [
 - "migrate"
 - "-json"
 - "-logFile filename"
]

As-is, we have to write a script and then call it like this

path: /flyway-log.sh

Or we have to in-line the command like this

path: /usr/bin/env sh
args: [
 - "-c"
 - "/flyway/flyway migrate json > filename"
]

The more truly native we can keep the calls the better. That way we don't have to worry about which terminals we have available, etc.

@juliahayward juliahayward removed this from the Flyway 6.1.1 milestone Dec 9, 2019
@juliahayward juliahayward added this to the Flyway 6.1.2 milestone Dec 9, 2019
@juliahayward
Copy link
Member

@juliahayward juliahayward commented Dec 9, 2019

Investigating this revealed a bug in writing the log which has been split off as #2595 .

@alextercete
Copy link
Contributor

@alextercete alextercete commented Dec 10, 2019

@esauser You make a good point: having an option to specify a file to which the output should be written makes for a more portable API.

Perhaps it would be useful if I shared a bit of the thought process we went through when imagining how the -logFile would work.

The original request was about allowing the output to be written to a file. Piping wasn't enough because there was a need to still have Flyway prompt for credentials, although this could still be achieved with the use of external tools, such as tee:

$ flyway info | tee out.txt
Password:

The disadvantage of external tools is that it would make the solution less portable, so we decided to introduce the -logFile option (which, in hindsight, maybe we should've called -outputFile).

Another aspect that weighed in favour of introducing the -logFile option is that it would allow having logs (particularly debug logs, when -X is provided) that would otherwise be missed when -json is provided. This ended up not being implemented as originally discussed, which has led to this issue.

To add to the equation, the codebase doesn't draw a very clear distinction between logging and user-facing output at the moment, which could lead to unexpected entries being written to the log (such as the ASCII table produced by flyway info).

@juliahayward
Copy link
Member

@juliahayward juliahayward commented Dec 11, 2019

OK, we've discussed and the conclusion we've come to is:

  • use -outputFile in preference to -logFile (though the latter will persist as a deprecated, undocumented option for people who picked it up early, at least for now)
  • when -json is turned on, you get nothing but the JSON object as output (a fatal error being part of the object). -X doesn't change this.
  • when -outputFile is turned on, the output file gets a verbatim copy of the console output regardless of -json

@alextercete
Copy link
Contributor

@alextercete alextercete commented Dec 11, 2019

Another thing is that -q should suppress output (apart for warnings and errors) from both the console and the file. However, all command-line options that set log level (namely -q and -X) should be ignored when -json is specified.

dohrayme pushed a commit to dohrayme/flyway that referenced this issue Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants