Skip to content
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

Issue 101 #152

Closed
wants to merge 24 commits into from
Closed

Issue 101 #152

wants to merge 24 commits into from

Conversation

alexsnaps
Copy link
Member

Shouldn't be too off...
For @timeck the ParsesConfigExtensionTest class contains my take on how to make this all work within our javax.cache.CacheManager implementation...

@terracotta-org
Copy link
Contributor

Test FAILed.

1 similar comment
@terracotta-org
Copy link
Contributor

Test FAILed.

@alexsnaps
Copy link
Member Author

test this please

@terracotta-org
Copy link
Contributor

Test FAILed.

@alexsnaps
Copy link
Member Author

No idea what's wrong! I'll let smart people tell me what I've done wrong...
I only have on thing to say: "Works on my machine!"

@timeck
Copy link
Contributor

timeck commented Nov 15, 2014

what didn't you change? :-)

@ljacomet
Copy link
Member

Most likely xjc failing due to spaces in the path ...

@alexsnaps
Copy link
Member Author

So @ljacomet ... no solution? Is it the plugin's fault or xic's?

@alexsnaps
Copy link
Member Author

@timeck Wanted to make sure this PR stays out for review at least a month! Thought that touching as many files as possible, preferably around generics, would help ;-)

@terracotta-org
Copy link
Contributor

Test FAILed.

@alexsnaps
Copy link
Member Author

Expiry is still missing in the XML stuff... Realized that on the plane while doing the doc stuff. I'll address this somehow, sometime, ...

@ljacomet
Copy link
Member

So the issue with jaxb/xjc comes from the gradle plugin - I opened issue scubacabra/gradle-jaxb-plugin#16
As the fix is rather simple, let's hope it means a release quickly.

jaxb {
xsdDir = "xml/src/main/resources"
episodesDir = "$buildDir/xsd/episodes"
bindingsDir = "$buildDir/xsd/bindings"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both dirs above are blindly appended to the project.rootDir, so they should be relative to it.

In this case, the value xml/build shoud replace the $buildDir variable.

@timeck
Copy link
Contributor

timeck commented Nov 17, 2014

Had a look over things here, nothing jumps out at me. Although I admit when I try to hook up 107 I might read a little more closely :-)

How does one configure the expiration settings for a Cache? Or is that what you were mentioning that isn't done yet?

@alexsnaps
Copy link
Member Author

Y forgot to wire the expiry in the config. Will try to address this ASAP, based on my availability here.

@@ -21,7 +21,15 @@
<xs:complexType name="config-type">
<xs:sequence>
<xs:element name="service" type="ehcache:service-type" minOccurs="0" maxOccurs="unbounded"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is an empty configuration file legal? Both the "service" element and the wrapper around the "cache" & "cache-template" elements have a minOccurs of zero. (Appears to be existing behavior in any case.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want an empty config to be valid. I currently have the 107 wrapper loading an ehcache-failsafe.xml resource when creating a 107 CacheManager and not providing a config URI

@chrisdennis
Copy link
Member

I put a couple of really minor comments in... I can't speak for the jaxb usage, and how canonical it is though.

@ljacomet ljacomet mentioned this pull request Nov 21, 2014
@ljacomet
Copy link
Member

PR #161 includes these changes it seems - asked for clarification there.

}

public static CacheManager newCacheManager(final Configuration configuration) {
return new EhcacheManager(configuration);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newCacheManager(ServiceLocator, Configuration) method calls init() on the CacheManager before returning. Should init() be called here as well?

@anthonydahanne
Copy link
Contributor

All :
During my work on issue #151 , I add to rebase Alex's 24 commits included in this PR.
Everything (the 24 commits) is in this PR : #161 along with few changes that fix the default eviction behavior.
See you in #161 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants