[infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

Galder Zamarreño
Hi all,

Given all the previous discussions we've had on this list [1] [2], seems like there's a majority of opinions towards moving Infinispan Hibernate 2LC cache provider to the Infinispan repo.

Although we could put it in a completely separate repo, given its importance, I think we should keep it in the main Infinispan repo.

With this in mind, I wanted to propose the following:

1. Move the code Hibernate repository and bring it to Infinispan master and 9.0.x branches. We'd need to introduce the module in the 9.0.x branch so that 9.0.x users are not left out.

2. Create a root directory called `hibernate-orm` within Infinispan main repo. Within it, we'd keep 1 or more cache provider modules based on major Hibernate versions.

3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?

4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.

5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?

6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?

Thoughts? Am I forgetting something?

Cheers,

[1] http://lists.jboss.org/pipermail/infinispan-dev/2017-February/017173.html
[2] http://lists.jboss.org/pipermail/infinispan-dev/2017-May/017546.html
--
Galder Zamarreño
Infinispan, Red Hat


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

Steve Ebersole
Just a heads up - FWIW I doubt my reply goes through to the entire infinispan-dev list.

Replies inline...


3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?

Since you use Maven (IIUC) you could just publish a relocation...
 

4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.

Probably, but (no promises) one thing I wanted to look at since Hibernate baselines on Java 8, is to maintain the existing SPI using default methods as a bridge.  But failing that, I think your suggestion is the best option.

 
5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?

FWIW Hibernate offers "short naming" or "friendly naming" for many configurable settings, cache providers being one.  For hibernate-infinispan we register 2: "infinispan" and "infinispan-jndi".  You can see this in org.hibernate.cache.infinispan.StrategyRegistrationProviderImpl.  That approach will continue to work when you move it.  The point being that users do not specify the class name in config, they'd just specify "infinispan", "infinispan-jndi", etc.
 

6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?

Hmmm, that is actually surprising... I thought the hibernate-infinispan  provider tests were still disabled as they had routinely caused intermittent failures of the build.  I guess this was rectified?


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

Galder Zamarreño
I think it's going through, we've approved you in the past.

Replies below:

> On 31 May 2017, at 17:02, Steve Ebersole <[hidden email]> wrote:
>
> Just a heads up - FWIW I doubt my reply goes through to the entire infinispan-dev list.
>
> Replies inline...
>
>
> 3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?
>
> Since you use Maven (IIUC) you could just publish a relocation...

Oh, didn't know about that. Yeah, I think we'd do that:
https://maven.apache.org/guides/mini/guide-relocation.html

>  
>
> 4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.
>
> Probably, but (no promises) one thing I wanted to look at since Hibernate baselines on Java 8, is to maintain the existing SPI using default methods as a bridge.  But failing that, I think your suggestion is the best option.
>
>  
> 5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?
>
> FWIW Hibernate offers "short naming" or "friendly naming" for many configurable settings, cache providers being one.  For hibernate-infinispan we register 2: "infinispan" and "infinispan-jndi".  You can see this in org.hibernate.cache.infinispan.StrategyRegistrationProviderImpl.  That approach will continue to work when you move it.  The point being that users do not specify the class name in config, they'd just specify "infinispan", "infinispan-jndi", etc.

Ah good to know, I wasn't aware of it. I'll look into that.

> 6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?
>
> Hmmm, that is actually surprising... I thought the hibernate-infinispan  provider tests were still disabled as they had routinely caused intermittent failures of the build.  I guess this was rectified?

They seem pretty stable to me when I run them locally.





_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

Steve Ebersole
Awesome, good to know...

About the tests... yes, that is the problem with "intermittent" ;)  You can run it 10 times and then it will break.  Then you run it 500 times and it breaks.  All without any code changes.  But like I said, if that is really fixed... awesome


On Fri, Jun 2, 2017 at 9:13 AM Galder Zamarreño <[hidden email]> wrote:
I think it's going through, we've approved you in the past.

Replies below:

> On 31 May 2017, at 17:02, Steve Ebersole <[hidden email]> wrote:
>
> Just a heads up - FWIW I doubt my reply goes through to the entire infinispan-dev list.
>
> Replies inline...
>
>
> 3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?
>
> Since you use Maven (IIUC) you could just publish a relocation...

Oh, didn't know about that. Yeah, I think we'd do that:
https://maven.apache.org/guides/mini/guide-relocation.html

>
>
> 4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.
>
> Probably, but (no promises) one thing I wanted to look at since Hibernate baselines on Java 8, is to maintain the existing SPI using default methods as a bridge.  But failing that, I think your suggestion is the best option.
>
>
> 5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?
>
> FWIW Hibernate offers "short naming" or "friendly naming" for many configurable settings, cache providers being one.  For hibernate-infinispan we register 2: "infinispan" and "infinispan-jndi".  You can see this in org.hibernate.cache.infinispan.StrategyRegistrationProviderImpl.  That approach will continue to work when you move it.  The point being that users do not specify the class name in config, they'd just specify "infinispan", "infinispan-jndi", etc.

Ah good to know, I wasn't aware of it. I'll look into that.

> 6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?
>
> Hmmm, that is actually surprising... I thought the hibernate-infinispan  provider tests were still disabled as they had routinely caused intermittent failures of the build.  I guess this was rectified?

They seem pretty stable to me when I run them locally.





_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

Sanne Grinovero-3
In reply to this post by Galder Zamarreño
A couple of comments inline:

On 2 June 2017 at 13:45, Galder Zamarreño <[hidden email]> wrote:
> I think it's going through, we've approved you in the past.
>
> Replies below:
>
>> On 31 May 2017, at 17:02, Steve Ebersole <[hidden email]> wrote:
>>
>> Just a heads up - FWIW I doubt my reply goes through to the entire infinispan-dev list.

We're good, I received your answers too.

>> Replies inline...
>>
>>
>> 3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?
>>
>> Since you use Maven (IIUC) you could just publish a relocation...
>
> Oh, didn't know about that. Yeah, I think we'd do that:
> https://maven.apache.org/guides/mini/guide-relocation.html

Here an example, from when the Hibernate Search / Infinispan Directory
component was moved to Infinispan:

 - https://github.com/hibernate/hibernate-search/blob/master/infinispan/pom.xml#L23-L30

I guess you'll need an example in Gradle, but at least it shows you
can redirect all of organizationId, artifactId and versions as this
has worked nicely.

It's transparent to the users but they'll get a warning in the build.

>> 4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.
>>
>> Probably, but (no promises) one thing I wanted to look at since Hibernate baselines on Java 8, is to maintain the existing SPI using default methods as a bridge.  But failing that, I think your suggestion is the best option.
>>
>>
>> 5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?
>>
>> FWIW Hibernate offers "short naming" or "friendly naming" for many configurable settings, cache providers being one.  For hibernate-infinispan we register 2: "infinispan" and "infinispan-jndi".  You can see this in org.hibernate.cache.infinispan.StrategyRegistrationProviderImpl.  That approach will continue to work when you move it.  The point being that users do not specify the class name in config, they'd just specify "infinispan", "infinispan-jndi", etc.
>
> Ah good to know, I wasn't aware of it. I'll look into that.
>
>> 6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?

I wouldn't convert them, unless you have a concrete need to reuse some
helpers, but even so I'd be conservative.
Several other modules within Infinispan are also using JUnit and this
is unlikely to change.

Eventually I'd actually like to discuss moving it all to JUnit 5, but
let's keep that debate for another day.

Thanks,
Sanne

>>
>> Hmmm, that is actually surprising... I thought the hibernate-infinispan  provider tests were still disabled as they had routinely caused intermittent failures of the build.  I guess this was rectified?
>
> They seem pretty stable to me when I run them locally.
>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Loading...