ISPN-7464 Make caffeine optional dependency for infinispan core #4856
Conversation
|
Wait to make sure build all passes so far |
wburns
changed the title
|
Updated commit to point to correct JIRA. Branch unfortunately is still old, left alone to keep PR as is. |
commons/pom.xml
Outdated
| @@ -17,6 +17,7 @@ | |||
| <dependency> | |||
| <groupId>com.github.ben-manes.caffeine</groupId> | |||
| <artifactId>caffeine</artifactId> | |||
| <optional>true</optional> | |||
There was a problem hiding this comment.
Can you clarify this. Would infinispan-commons work fine when caffeine is missing?
There was a problem hiding this comment.
It would work, except for the method CollectionFactory#makeBoundedConcurrentMap
There was a problem hiding this comment.
Although I am not sure if the CollectionFactory class itself would work since it has an import for Caffeine.
There was a problem hiding this comment.
In other words: it would be great if infinispan-commons would avoid having this dependency, then we'd have reassurances from the compiler.
There was a problem hiding this comment.
ic. Are there many use cases to have infinispan-commons, yet no have caffeine then?
If all (or most) uses will need this, one might as well keep it as a non-optional dependency.
| @@ -6,7 +6,7 @@ | |||
| <dependencies> | |||
| <module name="javax.api"/> | |||
| <module name="javax.transaction.api"/> | |||
| <module name="com.github.ben-manes.caffeine" slot="${slot}" export="true"/> | |||
| <module name="com.github.ben-manes.caffeine"/> | |||
There was a problem hiding this comment.
thanks, that is the line I was wondering about when I asked on IRC if that was necessary. Apparently not :)
There was a problem hiding this comment.
Yeah you can chalk that up to me not knowing what to do with wildfly-modules :)
There was a problem hiding this comment.
as discussed on IRC: the slot qualifier needs to stay.
client/hotrod-client/pom.xml
Outdated
| @@ -25,6 +25,11 @@ | |||
| </dependency> | |||
|
|
|||
| <dependency> | |||
| <groupId>com.github.ben-manes.caffeine</groupId> | |||
| <artifactId>caffeine</artifactId> | |||
| </dependency> | |||
There was a problem hiding this comment.
why does the client need caffeine?
There was a problem hiding this comment.
The Near cache in the client is a bounded concurrent map.
wburns
force-pushed
the
ISPN-7565
branch
2 times, most recently
from
10bcea5
to
0ecbd4f
Compare
|
Updated to leave maven pom alone. Also made sure slot is defined in case of different versions of Caffeine are deployed together |
|
I will also remove the modules from core and client to see if they pass tests without. Before I had to export them, maybe they are working without now. |
wburns
force-pushed
the
ISPN-7565
branch
2 times, most recently
from
1e33add
to
f39acdf
Compare
|
Seems the other imports are required, adding them back in. |
* Make caffeine internal dependency for as modules * Add osgi bundle for caffeine in commons
|
Last one looks good. I'll wait CI to have all results, as I only run a specific subset of tests. |
|
"All checks have failed" .. |
|
Pushed to master |
https://issues.jboss.org/browse/ISPN-7464