Don't thrown an exception if the JAR manifest does not specify the client version#1
Don't thrown an exception if the JAR manifest does not specify the client version#1celkins wants to merge 1 commit into
Conversation
|
The NOT-FROM-JAR was originally only for testing purposes (it should be removed). The version number however is mostly required: we perform server logic based on the client versions for certain backwards compatibility concerns. It is also planned to use it in the near future for customizing media type versions served by the REST API. Might there be another way to get this information with the Shade plugin? Perhaps create a 'Stormpath-Sdk-Implementation-Version' manifest entry that you can populate into your aggregate jar? Then we can look for that first and fall back to 'Implementation-Version' if it doesn't exist. Thoughts? |
|
Also, we're open to other techniques for this as well - e.g. maybe populating a stormpath.sdk.version.properties file via Maven filtering at build time and then look that up at runtime. Its just that the Manifest is a particularly elegant way to retain .jar metadata (that is its purpose after all), which is why we went with that approach. We're very much open to suggestions. |
|
Yes, this change is certainly inappropriate if semantics are applied to the client version string. Since I'm not using my own manifest file for any purpose, I can work around the issue by adding the expected entry to my shaded JAR. However, support for an SDK-specific technique would be welcome, because declaring an incorrect implementation version for my own JAR is misleading. How about using the standard manifest entry as the primary and falling back to a properties file in the resource path if necessary? |
|
Hi Chris - do you not want us to try the version.properties file approach? (I was using this issue as a placeholder for that). |
|
Yes, please do. (Sorry. I meant to close the pull request and leave the issue open, but I guess their state is shared.) FYI in case you haven't seen how I'm using the SDK: https://github.com/celkins/dropwizard-stormpath |
|
Chris, this is super cool! Thanks so much for sharing - definitely let us know if you have any questions along the way. |
|
This is also an issue if you bundle it in a spring-boot fat jar see: spring-projects/spring-boot#92 Doing something like: String version = Version.class.getPackage().getImplementationVersion();Works pretty good not sure if it would in jar created by the shade plugin since it flattens all jar deps contents into a single jar (if i'm not mistaken) but this code snippet covers all normal cases without assuming anything about the resources URL as pointed out by Dave Syer in the spring-boot ticket I created. By the way @lhazlewood don't know if you remember me from Roundbox (trip to Ireland with Jeff and Chris) ;) |
|
@jarias Hi Julio! Of COURSE I remember you! We had a ton of great times on that trip! And thanks for the code snippet :) Are you using the Stormpath SDK for anything? |
|
I got bit by this over the weekend when using Maven's 'jar-with-dependencies' artifact for an executable .jar. Definitely need the version.properties approach. |
|
Closing - this is superseded by Issue 36 |
If the Stormpath client classes are embedded within another JAR (e.g., using the Maven shade plugin) and the original manifest file is not included, the client version lookup heuristic fails with an error. It seems that this version string is purely informational, given that lookupClientVersion() returns "NOT-FROM-JAR" under similar circumstances.
I have no opinion on the correct string to return in such cases, but throwing an exception seems too severe.