Add Rule Logger #1040

ddcruver · 2019-11-05T21:12:27Z

No description provided.

…nd assign them to the RuleFactory through the Jsonschema2Pojo#generate method.

…e all together. Now it adds the correct classpathPrefix to import all libraries from a subdirectory lib which is where copy-dependencies puts them and in general that is good practice if you were to distribute this tool.

ddcruver · 2019-11-05T21:17:28Z

Opps .. tests need logger now... actually it looks like the test itself maybe failed. Added some mocking to RuleFactory in a few tests. But problem may be elsewhere.

ddcruver · 2019-11-05T22:15:36Z

@joelittlejohn
It looks like the Gradle version (1.6) it builds the code does not match the version (2.3) that is run on the build server. Which version of Gradle should we target? Both versions are ancient.

jsonschema2pojo-ant/src/main/java/org/jsonschema2pojo/ant/AntRuleLogger.java

jsonschema2pojo-cli/pom.xml

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bash

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.sh

jsonschema2pojo-cli/src/main/scripts/simplelogger.properties

joelittlejohn · 2019-11-05T22:32:07Z

Strange, I have no idea why it says that this zip file was empty:

org.gradle.tooling.GradleConnectionException: Could not install Gradle distribution from 'http://services.gradle.org/distributions/gradle-2.3-bin.zip'.

When I download this using the same link I don't get an empty file.

ddcruver · 2019-11-06T04:07:26Z

I think the stack trace above it gives more hint at what is going on but still not sure how to fix it.

Are you open to updating the groovy, Gradle, groovy maven plugin to resolve this issue? Or I could just not support logging under the Gradle plugin.

ddcruver · 2019-11-06T14:47:03Z

The gradle plugin project failing is a weird problem because it fails only on the Travis CI server and not a local maven execution of it.

…e already set to test scope by dependency management.

…ment

… directory. Added .gitattributes to alwasy checkout shell scripts with linux line endings. Removed .bash version because it really is not needed.

…ing.GradleConnectionException: Could not install Gradle distribution from 'http://services.gradle.org/distributions/gradle-2.3-bin.zip problem'.

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat

jsonschema2pojo-gradle-plugin/src/test/resources/simplelogger.properties

joelittlejohn · 2019-11-06T18:46:19Z

.travis.yml

+script:
+  - rm -rf ~/.gradle/wrapper/dists/gradle-2.3-bin
+  - mvn clean install -DskipTests
+  - mvn verify


What's the reason for install then verify?

I was trying to resolve the build issues and found this commit (81adfe7) and it mentioned that CI tests might be broken unless do a build then do a test build.

@ben-manes Is this unnecessary now?

This might be necessary with the eclipse-groovy-compiler. I will try removing it.

That commit comment wrt CI failure was a one-time breakage and no longer relevant. Just required a clean if I recall correctly.

@joelittlejohn I have already removed the extra build and haven't notice any problems.
@ben-manes Thanks for your input.

So I think all the changes in this file can be reverted, is that right?

I have removed the hacky gradle bin and double building stuff from Travis CI configuration but left in the caching.

ddcruver · 2019-11-06T18:53:24Z

@joelittlejohn Since #1041 is also failing I believe this failure is not a result of my changes. Something is broken with the build.

This is a far reach but @ben-manes do you have any insight into this problem?

ben-manes · 2019-11-06T18:59:37Z

I believe it is due to not following a redirect. They enforce HTTPS now and their http client probably doesn't have redirect enabled. So maybe a 1 character fix! 😄

$ curl -I http://services.gradle.org/distributions/gradle-2.3-bin.zip
HTTP/1.1 301 Moved Permanently
Date: Wed, 06 Nov 2019 18:58:00 GMT
Connection: keep-alive
Cache-Control: max-age=3600
Expires: Wed, 06 Nov 2019 19:58:00 GMT
Location: http://downloads.gradle-dn.com/distributions/gradle-2.3-bin.zip
Server: cloudflare
CF-RAY: 53194be1fed592c8-SJC

ben: lucene-solr $ wget -v http://services.gradle.org/distributions/gradle-2.3-bin.zip
--2019-11-06 10:58:10--  http://services.gradle.org/distributions/gradle-2.3-bin.zip
Resolving services.gradle.org (services.gradle.org)... 104.18.190.9, 104.18.191.9
Connecting to services.gradle.org (services.gradle.org)|104.18.190.9|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://downloads.gradle-dn.com/distributions/gradle-2.3-bin.zip [following]
--2019-11-06 10:58:10--  http://downloads.gradle-dn.com/distributions/gradle-2.3-bin.zip
Resolving downloads.gradle-dn.com (downloads.gradle-dn.com)... 104.17.160.20, 104.17.159.20
Connecting to downloads.gradle-dn.com (downloads.gradle-dn.com)|104.17.160.20|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: https://downloads.gradle-dn.com/distributions/gradle-2.3-bin.zip [following]
--2019-11-06 10:58:10--  https://downloads.gradle-dn.com/distributions/gradle-2.3-bin.zip
Connecting to downloads.gradle-dn.com (downloads.gradle-dn.com)|104.17.160.20|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 42475057 (41M) [application/zip]
Saving to: ‘gradle-2.3-bin.zip’

gradle-2.3-bin.zip                       100%[==================================================================================>]  40.51M  20.2MB/s    in 2.0s

2019-11-06 10:58:12 (20.2 MB/s) - ‘gradle-2.3-bin.zip’ saved [42475057/42475057]

ddcruver · 2019-11-06T19:03:34Z

I literally asked you then thought to do that myself. Lets see if it works.

ddcruver · 2019-11-06T19:37:47Z

@ben-manes I also switch back to the groovy-eclipse-compiler, these days which groovy plugin should you use?

I have read lots of write-ups on "which maven plugin/compiler for groovy should I use" but not being a groovy developer I am still not sure what this project should be using. I changed it because I thought it might fix my build/execution issues but it seems like it might of been unrelated now.

ben-manes · 2019-11-06T19:45:43Z

I can't answer at that detail. I simply hacked together a few plugins for my needs and contributed them upstream (flyway, js2p, jooq, dependencyUpdates). I learned enough Groovy to write simple build scripts and plugins, but this was Gradle 1.x world (2012). I don't use Groovy otherwise and tried to handoff the plugins since this isn't my area of expertise. I'm happy to be a sounding board but my knowledge is fairly limited and outdated.

ddcruver · 2019-11-06T20:00:47Z

I read that the gmaven(plus) plugins had more features and may support more things but I believe thease are things we don't require, so since the eclipse plugin is an official plugin we should use it instead.

I believe there is other improvements to the jsonschema2pojo-gradle-plugin and maybe it should actually be build with gradle (embedded in our maven) instead but that is out of scope with something I am willing to do right now.

ddcruver · 2019-11-06T20:10:39Z

@joelittlejohn I believe I have responded and/or addressed all your comments and the build looks good now.

joelittlejohn · 2019-11-06T20:33:36Z

Great, thanks for looking at the Gradle build issues. I think we still have a few changes we need to make here, in particular any change that removes ../lib from CLI scripts needs to be reverted. I've added some follow-up comments and unresolved a few conversations.

Also, is there any reason the CLI logger needs to use SLF4J rather than just System.out and System.err? This will never be part of a wider system with its own logging configuration.

ddcruver · 2019-11-06T20:52:04Z

Also, is there any reason the CLI logger needs to use SLF4J rather than just System.out and System.err? This will never be part of a wider system with its own logging configuration.

I guess if this was jsonschema2-pojo-embedded (a fictional module) that would be integrated in another application/system then it should use SLF4J but I agree. Although there would be no way to turn off loggers for CLI if people started using info/debug/trace and it could get quit noisy.

UPDATE: I updated CLI to support command line provided log level.

…oid breaking the release.

joelittlejohn · 2019-11-06T21:37:39Z

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat

+@REM
+
+@echo off
+java -jar "%~dp0/../lib/${project.build.finalName}.jar" %*


Your branch is still showing a change in this file from CRLF line endings to LF. Could you maybe git checkout origin/master jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat and commit that?

Thanks, it was CRLF on my machine and source tree wasn't showing any changes. I had to manually remove the file and check it out like you suggested then add it via command line git commands.

I have added .bat to the .gitattributes file and renormlized that bat file. It may still show changes because it was original in the git repo as LF.

https://stackoverflow.com/a/47580886/45708

Hi Dan. Everything looks good but this is the last problem in the PR. In master this file uses CRLF (Windows) line endings not LF:

$ file jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat: DOS batch file, UTF-8 Unicode text, with CRLF line terminators

You can also try vim -b jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat to see the ^M characters.

In your branch (this PR) changes the line endings in this file to LF.

I've actually changed my mind about this .gitattributes file, it's adding more confusion and I don't want git doing anything to change the files under version control. Please remove .gitattributes and also remove your changes to this file (git checkout origin/master jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat). Then we're good to go.

…mic what SLFJ would provide. Impelment log level functionality to CLI so it does not get flooded with high verbose logging especially if debug and verbos log levels are used.

… Gradle download problem.

…ption is choosen it will exit after printing log levels.

…treated as CRLF.

… going to use default configuration for now.

ddcruver · 2019-11-07T16:34:32Z

@joelittlejohn I am ready for you to take another pass of seeing if this pull request is ready to merge.

ddcruver · 2019-11-19T14:14:46Z

@joelittlejohn I would like to wrap up this pull request and possibly #1041. I would like to write a RuleFactory with a custom EnumRule for my project that has the name fallback resolution as described in #999 and I want it to extend EnumRule instead of re-implementing it.

ddcruver added 4 commits November 5, 2019 15:56

joelittlejohn#113 Create RuleLogger interface and implementations.

ed63237

joelittlejohn#113 Create RuleLogger implementation for each factory a…

b92a9ab

…nd assign them to the RuleFactory through the Jsonschema2Pojo#generate method.

joelittlejohn#113 Move some test dependencies out of the compile scope.

c4ffa94

ddcruver changed the title ~~Feature/add rule logger~~ Add Rule Logger Nov 5, 2019

ddcruver added 2 commits November 5, 2019 16:41

joelittlejohn#113 Add RuleLogger mocking to some tests.

71c4632

joelittlejohn#113 Add SLF4J binding for gradle plugin tests.

e8f542c

joelittlejohn reviewed Nov 5, 2019

View reviewed changes

joelittlejohn mentioned this pull request Nov 5, 2019

Internals - Add logging functionality to RuleFactory #1039

Closed

ddcruver added 11 commits November 6, 2019 10:20

joelittlejohn#1040 Remove explicit scope in test dependencies that ar…

82c04fd

…e already set to test scope by dependency management.

joelittlejohn#1040 Use slf4j instead of gradle logger

fc3e4da

joelittlejohn#1040 Use slf4j instead of gradle logger - fix log state…

ab83f5c

…ment

joelittlejohn#1040 Move simplelogger.properties to classpath.

1cb089f

joelittlejohn#1040 Try to get CI Build working.

5867a0e

joelittlejohn#1040 Try to get CI Build working. Upgrade Groovy version.

c272508

joelittlejohn#1040 Remove pushd/popd to not mess with current working…

34a6888

… directory. Added .gitattributes to alwasy checkout shell scripts with linux line endings. Removed .bash version because it really is not needed.

joelittlejohn#1040 Try deleting gradle-2.3-bin to fix org.gradle.tool…

1772deb

…ing.GradleConnectionException: Could not install Gradle distribution from 'http://services.gradle.org/distributions/gradle-2.3-bin.zip problem'.

joelittlejohn#1040 Fix outdated references and spelling errors.

bd6c9a0

joelittlejohn#1040 Try to get CI Build working.

2faea92

joelittlejohn#1040 Try to get CI Build working. Using eclipse plugin.

f70d0cf

joelittlejohn reviewed Nov 6, 2019

View reviewed changes

jsonschema2pojo-cli/src/main/scripts/jsonschema2pojo.bat Outdated Show resolved Hide resolved

joelittlejohn reviewed Nov 6, 2019

View reviewed changes

jsonschema2pojo-gradle-plugin/src/test/resources/simplelogger.properties Outdated Show resolved Hide resolved

joelittlejohn reviewed Nov 6, 2019

View reviewed changes

joelittlejohn#1040 Try to get CI Build working. Force it to use https.

2ad7943

joelittlejohn#1040 Switch back to using Gradle framework logger.

268ff15

joelittlejohn#1040 Try build without to maven builds.

4e8933c

ddcruver added 2 commits November 6, 2019 16:01

joelittlejohn#1040 Remove switch in AntRuleLogger to address PR comment.

1605a54

joelittlejohn#1040 Revert changes to jars classpath and scripts to av…

6328cdb

…oid breaking the release.

joelittlejohn reviewed Nov 6, 2019

View reviewed changes

ddcruver added 9 commits November 7, 2019 09:55

joelittlejohn#1040 Remove SLFJ from CLI module.

eb612ac

joelittlejohn#1040 Add is enable checks to RuleLogger interface to mi…

a6174e1

…mic what SLFJ would provide. Impelment log level functionality to CLI so it does not get flooded with high verbose logging especially if debug and verbos log levels are used.

joelittlejohn#1040 Fix some instances of hardcoded "UTF-8"

154b3db

joelittlejohn#1040 Cleanup Travis CI integration since we resolve the…

156f35e

… Gradle download problem.

joelittlejohn#1040 Convert bat file back to Windows line endings.

d363ccc

joelittlejohn#1040 Add note in description that if print-log-levels o…

a069661

…ption is choosen it will exit after printing log levels.

joelittlejohn#1040 Add Batch files to .gitattributes so it is always …

63a1a70

…treated as CRLF.

joelittlejohn#1040 Remove simplelogger.properties because we are just…

44472e8

… going to use default configuration for now.

joelittlejohn#1040 Renormalize jsonschema2pojo.bat

222d2fb

joelittlejohn#1040 Remove .gitattributes and revert jsonschema2pojo.bat.

646d9f8

joelittlejohn merged commit 4fa79b5 into joelittlejohn:master Nov 19, 2019

ddcruver deleted the feature/add-rule-logger branch November 19, 2019 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rule Logger #1040

Add Rule Logger #1040

ddcruver commented Nov 5, 2019

ddcruver commented Nov 5, 2019 •

edited

Loading

ddcruver commented Nov 5, 2019

joelittlejohn commented Nov 5, 2019 •

edited

Loading

ddcruver commented Nov 6, 2019

ddcruver commented Nov 6, 2019

joelittlejohn Nov 6, 2019

ddcruver Nov 6, 2019

ddcruver Nov 6, 2019

ben-manes Nov 6, 2019

ddcruver Nov 6, 2019

joelittlejohn Nov 6, 2019

ddcruver Nov 7, 2019

ddcruver commented Nov 6, 2019

ben-manes commented Nov 6, 2019

ddcruver commented Nov 6, 2019 •

edited by joelittlejohn

Loading

ddcruver commented Nov 6, 2019

ben-manes commented Nov 6, 2019

ddcruver commented Nov 6, 2019 •

edited

Loading

ddcruver commented Nov 6, 2019 •

edited

Loading

joelittlejohn commented Nov 6, 2019

ddcruver commented Nov 6, 2019 •

edited

Loading

joelittlejohn Nov 6, 2019

ddcruver Nov 7, 2019

ddcruver Nov 7, 2019

joelittlejohn Nov 15, 2019

ddcruver Nov 15, 2019

ddcruver commented Nov 7, 2019

ddcruver commented Nov 19, 2019

Add Rule Logger #1040

Add Rule Logger #1040

Conversation

ddcruver commented Nov 5, 2019

ddcruver commented Nov 5, 2019 • edited Loading

ddcruver commented Nov 5, 2019

joelittlejohn commented Nov 5, 2019 • edited Loading

ddcruver commented Nov 6, 2019

ddcruver commented Nov 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddcruver commented Nov 6, 2019

ben-manes commented Nov 6, 2019

ddcruver commented Nov 6, 2019 • edited by joelittlejohn Loading

ddcruver commented Nov 6, 2019

ben-manes commented Nov 6, 2019

ddcruver commented Nov 6, 2019 • edited Loading

ddcruver commented Nov 6, 2019 • edited Loading

joelittlejohn commented Nov 6, 2019

ddcruver commented Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddcruver commented Nov 7, 2019

ddcruver commented Nov 19, 2019

ddcruver commented Nov 5, 2019 •

edited

Loading

joelittlejohn commented Nov 5, 2019 •

edited

Loading

ddcruver commented Nov 6, 2019 •

edited by joelittlejohn

Loading

ddcruver commented Nov 6, 2019 •

edited

Loading

ddcruver commented Nov 6, 2019 •

edited

Loading

ddcruver commented Nov 6, 2019 •

edited

Loading