-
Notifications
You must be signed in to change notification settings - Fork 24
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
UPSE-293: updates to support Java 11 #269
Conversation
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 @groybal!
Logback and Jaxb changes look good!
Have a few questions on some of the other changes in here.
@@ -27,7 +27,7 @@ | |||
<!-- http://www.qos.ch/shop/products/professionalSupport --> | |||
<!-- --> | |||
<configuration scan="true" scanPeriod="30 seconds"> | |||
<contextName>ResourceServingWebapp</contextName> | |||
<contextName>resource-server</contextName> |
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.
was this needed as part of the log back downgrade?
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.
We have two resource server webapps running, an older one (ResourceServingWebapp version 1.0.48) and a newer one (resource-server version 1.4.x) but they both log to the same log file so it was confusing to debug issues. I can remove this change if we still want both webapps to log to same file. Plus new naming matches the newer webapp name (resource-server).
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.
Ahh, gotcha, that sounds like a good idea. 👍
Just wanted to see how it fit with the other changes.
@@ -158,7 +170,7 @@ | |||
<executions> | |||
<execution> | |||
<id>sign-artifacts</id> | |||
<phase>verify</phase> | |||
<phase>deploy</phase> |
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.
Is this related to the jaxb/logback changes?
Is this needed for Java 11?
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.
If I'm remembering correctly, with 'verify', the build was failing with Java 11. I changed it to 'deploy' to resolve because I figured we don't need to sign artifacts anyways unless we're doing a deploy. Now that I think about it, if we run a deploy the same error will probably still occur :(
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.
Okay so I retested using 'verify' and I get build failure with both Java 8 and Java 11. Here's the error, anyone have insights:
[DEBUG] 2.2.19
[GNUPG:] INV_SGNR 9
[GNUPG:] FAILURE sign 17
gpg: no default secret key: No secret key
gpg: signing failed: No secret key
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.
I'd check
gpg --list-keys
to see if there is a key already setup on your system
If there isn't try running
gpg --generate-key
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.
Yep, that fixed it (generating key). Still should use 'deploy' I think but let me know if I should change back to 'verify'.
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.
I'm good with it staying in deploy as well.
Though open to differing opinions.
updating to the latest Java 8 compatible version logback 1.3.5
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.
This works great! I had to revise the logback version but it's working. Thanks
Added JAXB dependencies for Java 11.
Downgraded Logback version to one that runs with Java 8 (tricky: runtime error only -- no compile error).