[GitHub] brooklyn-server issue #339: Updated jackson version

2016-09-27 Thread aledsage
Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/339
  
LGTM; merging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] brooklyn-server issue #339: Updated jackson version

2016-09-27 Thread Graeme-Miller
Github user Graeme-Miller commented on the issue:

https://github.com/apache/brooklyn-server/pull/339
  
I have tested this with vanilla/karaf brooklyn and karaf AMP.
I have tested deploying a blueprint and using the Swagger API.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] brooklyn-server issue #339: Updated jackson version

2016-09-27 Thread Graeme-Miller
Github user Graeme-Miller commented on the issue:

https://github.com/apache/brooklyn-server/pull/339
  
We have decided to proceed with updating the dependency.
We have also added a commit recommended by Svet in the predecessor to this 
PR: 
https://github.com/neykov/brooklyn-server/commit/62613c05c2ba08c3ad9055d1fa3006f100a58b38

Lastly Swagger has been forbidden from pulling in any jackson versions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] brooklyn-server issue #339: Updated jackson version

2016-09-25 Thread aledsage
Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/339
  
@sjcorbett observed in https://github.com/apache/brooklyn-server/pull/317 
that io.swagger pulls in 2.4.5 of dataformat-jackson-dataformat-yaml etc.

@neykov said:

```
That's a good spot @sjcorbett. The jackson library version mix seems 
important. 
I think it's better to stick to the same version (even if higher than 
required 
for swagger), than to risk incompatibility. It's higher risk to use two 
parts 
of the library with different versions than upgrade all of them.

Another reason to upgrade everything is that in Karaf we are explicitly 
adding 
the dependencies and pinning them to the fasterxml.jackson.version. This 
causes 
the classic distribution to have a different mix of jars than the karaf one 
- 
best to keep them consistent.
```

---
Even in karaf, when using brooklyn-tosca-karaf-init, we'll get the same mix 
of class versions: jackson-datatype-joda-2.4.5.jar along with the 
jackson-annotations bundle version 2.7.5.

I'm not sure how safe that version mix is. But I'm also not sure how safe 
it is to upgrade `datatype-jackson-datatype-joda` etc to 2.7.5 (e.g. will that 
be binary compatible with swagger 2.4.5, or will it break in any code paths?).

The options seem to be:

1. Reject this PR (and force downstream projects to figure out how to cope 
with Brooklyn using an old version of jackson-core).

2. Just upgrade jackson to 2.7.5 (as proposed in this PR): trust our QA, 
and trust jackson to be doing semantic versioning.

3. Build a custom swagger-core 1.5.6 OSGi bundle, so it depends on exactly 
jackson 2.4.5 (and thus have both versions safely available in Karaf)

And we could also optionally:

4. Have brooklyn-tosca-karaf-init also bundle `jackson-core-2.4.5.jar`

Option (3) sounds horrible.

Option (1) passes the pain onto downstream projects.  Even if using Karaf, 
if that downstream project was to cause jackson-core 2.7.5 bundle to be 
included in Karaf, then swagger would immediately pick it up instead (because 
it depends on import version `[2.4,3)`). It could try including the jars in the 
OSGi bundle's `Bundle-ClassPath`, but that's annoying to have to do!

I'd be willing to risk option (2) and merge this as-is, given we've tested 
it. Also swagger have declared jackson dependency as `[2.4,3)` so hopefully it 
means they trust jackson compatibility!

I hate figuring out the right dependency versions!

---
Investigating the dependencies, 
`brooklyn-dist/dist/target/brooklyn-dist/brooklyn` has:

```
./lib/brooklyn/com.fasterxml.jackson.core-jackson-annotations-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.core-jackson-core-2.7.5.jar
./lib/brooklyn/com.fasterxml.jackson.core-jackson-databind-2.7.5.jar

./lib/brooklyn/com.fasterxml.jackson.dataformat-jackson-dataformat-xml-2.4.5.jar

./lib/brooklyn/com.fasterxml.jackson.dataformat-jackson-dataformat-yaml-2.4.5.jar

./lib/brooklyn/com.fasterxml.jackson.datatype-jackson-datatype-joda-2.4.5.jar
./lib/brooklyn/com.fasterxml.jackson.jaxrs-jackson-jaxrs-base-2.7.5.jar

./lib/brooklyn/com.fasterxml.jackson.jaxrs-jackson-jaxrs-json-provider-2.7.5.jar

./lib/brooklyn/com.fasterxml.jackson.module-jackson-module-jaxb-annotations-2.7.5.jar
```

Whereas `brooklyn-dist/karaf/apache-brooklyn/target/assembly` has:

```

./system/com/fasterxml/jackson/core/jackson-annotations/2.7.5/jackson-annotations-2.7.5.jar

./system/com/fasterxml/jackson/core/jackson-core/2.7.5/jackson-core-2.7.5.jar

./system/com/fasterxml/jackson/core/jackson-databind/2.7.5/jackson-databind-2.7.5.jar

./system/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.7.5/jackson-dataformat-yaml-2.7.5.jar

./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-base/2.7.5/jackson-jaxrs-base-2.7.5.jar

./system/com/fasterxml/jackson/jaxrs/jackson-jaxrs-json-provider/2.7.5/jackson-jaxrs-json-provider-2.7.5.jar
```

Looking at a downstream karaf that also bundles the `brooklyn-tosca` 
feature (with the bundle `brooklyn-tosca-karaf-init/0.10.0-SNAPSHOT`), the 
system dir has:

```

./system/com/fasterxml/jackson/core/jackson-annotations/2.4.5/jackson-annotations-2.4.5.jar

./system/com/fasterxml/jackson/core/jackson-annotations/2.7.5/jackson-annotations-2.7.5.jar

./system/com/fasterxml/jackson/core/jackson-core/2.4.5/jackson-core-2.4.5.jar

./system/com/fasterxml/jackson/core/jackson-core/2.7.5/jackson-core-2.7.5.jar

./system/com/fasterxml/jackson/core/jackson-databind/2.4.5/jackson-databind-2.4.5.jar

./system/com/fasterxml/jackson/core/jackson-databind/2.7.5/jackson-databind-2.7.5.jar

./system/com/fasterxml/jackson/dataformat/jackson-dataformat-yaml/2.4.5/jackson-dataformat-y

[GitHub] brooklyn-server issue #339: Updated jackson version

2016-09-25 Thread aledsage
Github user aledsage commented on the issue:

https://github.com/apache/brooklyn-server/pull/339
  
As stated by @Graeme-Miller in #317, the reason for the version bump is 
that another downstream project has a dependency on 
https://github.com/fabric8io/kubernetes-client which requires the later version.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---