[infinispan-dev] Hibernate Search integration modules for WildFly have been broken

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[infinispan-dev] Hibernate Search integration modules for WildFly have been broken

Sanne Grinovero-3
Hi all,

I was going to give ISPN-8779 a shot but I'm finding a mess.

the root pom contains these twice (and inconsistent!):

<version.hibernate.search>5.8.1.Final</version.hibernate.search>
[...]
<version.hibernate.search>5.8.0.Final</version.hibernate.search>

the BOM cointains a copy of `version.hibernate.search` as well.

I don't mind deleting duplicate properties, but we used to have
clearly separate properties for different purposes, and this
separation is essential.

I've mentioned this multiple times when reviewing PRs which would get
my attention, but I didn't see these changes - certainly didn't expect
you all to forget the special purpose of these modules.
It's quite messy now and I'm honestly lost myself at how I could revert it.

In particular this module is broken now as it's targeting the wrong slot:
 - https://github.com/infinispan/infinispan/blob/master/wildfly-modules/src/main/resources/org/infinispan/for-hibernatesearch-wildfly/main/module.xml#L27

Clearly it's not consistent with the comment I've put on the module descriptor.
I don't see that module being included in the released modules either,
and clearly the integration tests didn't catch it because they have
been patched to use the wrong modules too :(

Other essential integration tests which I had put in place to make
sure they'd get your attention in case someone had such an idea.. have
been deleted.

Opening ISPN-8780, I would consider this a release blocker.

Thanks,
Sanne

See also:
 - https://github.com/infinispan/infinispan/blob/master/integrationtests/as-lucene-directory/READ.ME
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Hibernate Search integration modules for WildFly have been broken

Sanne Grinovero-3
Hi all,

spent friday and a good deal of the weekend exploring some options;
finally I have a proposal.
 - https://github.com/infinispan/infinispan/pull/5766

But let me give some background:

I was originally planning to upgrade Infinispan to Hibernate Search
5.9.0.Final only in Infinispan 9.3, as while the code itself didn't
change much, I was expecting quite some work in the area of the build
of WildFly modules.

However since the module structure in the current master is lacking
[ISPN-8780] I'm proposing now to do this upgrade already, so to not
have to refactor the build of modules now and then again shortly.

As a reminder, we want Infinispan to be able to provide a "Lucene
Directory Provider" for 3 versions of Hibernate Search:
 A) the version which is included in latest WildFly stable release
 B) the version which Infinispan Query is using
 C) the latest stable version

It is of course possible - even likely - that some of these versions
happen to be the same for a particular release train; that's nice
however please remember that while they might be *coincidentally* the
same, that's no good reason to remove the capability of the build
system to support these different versions when they happen to
diverge. That's why the properties which mark each of these versions
was strictly separate - and more crucially was not "redundant".

In the specific case, I was looking for the properties I needed to
change to make sure we could support latest Hibernate Search release
but they were gone, my guess is this was caused by the fact that
recently version[B] and version[C] happened to match so this confused.

At this stage I don't think it's worth it to try find and identify all
changes which should be reverted, as with the upcoming upgrade to
Hibernate Search 5.9.0.Final several changes in how we build modules
would be needed, so I'll send a PR to upgrade to HS 5.9.0.Final
already.

Risks and implications?

HS 5.9 is mostly the same as 5.8, the main difference for end users is
the integration with a different version of Hibernate ORM now
supporting JPA 2.2 - but this isn't relevant for Infinispan so I
consider the upgrade overall a low risk.
The other relevant difference is that we no longer publish a zip of
the modules for WildFly - we publish instead a set of rather fine
grained feature packs. This is good news for Infinispan as we can
better cherry pick which components you actually want, but it means
the build needs to be adapted now to deal with these feature packs.

In my PR for ISPN-8779 I'll use the Provisioning plugin for Maven to
materialize the modules that Infinispan needs, so that for now they
can be re-packaged into the zip files - so to not have any impact on
Infinispan users. I hope in a second stage you'll all see the benefits
of distributing feature packs so we'll be able to simplify some
things.

Feature packs normally have a notion of dependency to other feature
packs, but to keep the adaptation into "old style fat zip" I'll
reconfigure the provisioning task to disable transitive dependencies;
the drawback is that I'll have to declare the version of each feature
pack we need explicitly in the parent pom: we'll be able to avoid the
need to match the dependant versions when Infinispan also will produce
feature packs rather than the fat zip.

N.B. this PR #5766 doesn't address the fact that the build doesn't
differentiate between the above cases B and C, but since B and C would
just happen to be the same version again in Infinispan 9.2 the issue
is no longer urgent, so ISPN-8780 could be postponed to 9.3+ and it
might be much easier after migrating some Infinispan modules to
feature packs as well.

Thanks,
Sanne


On 7 February 2018 at 16:59, Sanne Grinovero <[hidden email]> wrote:

> Hi all,
>
> I was going to give ISPN-8779 a shot but I'm finding a mess.
>
> the root pom contains these twice (and inconsistent!):
>
> <version.hibernate.search>5.8.1.Final</version.hibernate.search>
> [...]
> <version.hibernate.search>5.8.0.Final</version.hibernate.search>
>
> the BOM cointains a copy of `version.hibernate.search` as well.
>
> I don't mind deleting duplicate properties, but we used to have
> clearly separate properties for different purposes, and this
> separation is essential.
>
> I've mentioned this multiple times when reviewing PRs which would get
> my attention, but I didn't see these changes - certainly didn't expect
> you all to forget the special purpose of these modules.
> It's quite messy now and I'm honestly lost myself at how I could revert it.
>
> In particular this module is broken now as it's targeting the wrong slot:
>  - https://github.com/infinispan/infinispan/blob/master/wildfly-modules/src/main/resources/org/infinispan/for-hibernatesearch-wildfly/main/module.xml#L27
>
> Clearly it's not consistent with the comment I've put on the module descriptor.
> I don't see that module being included in the released modules either,
> and clearly the integration tests didn't catch it because they have
> been patched to use the wrong modules too :(
>
> Other essential integration tests which I had put in place to make
> sure they'd get your attention in case someone had such an idea.. have
> been deleted.
>
> Opening ISPN-8780, I would consider this a release blocker.
>
> Thanks,
> Sanne
>
> See also:
>  - https://github.com/infinispan/infinispan/blob/master/integrationtests/as-lucene-directory/READ.ME
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Hibernate Search integration modules for WildFly have been broken

Ryan Emerson-2
Hi Sanne,

Thanks for the input and the proposed solution. I am not familiar
with Hibernate search and our integrations, so I will leave others to
comment on the version upgrade. I just wanted to let you know that the
transition to feature-packs is on the roadmap for 9.3 as part of our
ongoing server refactoring efforts. Furthermore, I believe the breaking
of the wildfly-modules was caused by my work on the bom and parent pom
refactoring, so apologies for the inconvenience, I owe you a beer!

Cheers
Ryan

----- Original Message -----

> Hi all,
>
> spent friday and a good deal of the weekend exploring some options;
> finally I have a proposal.
>  - https://github.com/infinispan/infinispan/pull/5766
>
> But let me give some background:
>
> I was originally planning to upgrade Infinispan to Hibernate Search
> 5.9.0.Final only in Infinispan 9.3, as while the code itself didn't
> change much, I was expecting quite some work in the area of the build
> of WildFly modules.
>
> However since the module structure in the current master is lacking
> [ISPN-8780] I'm proposing now to do this upgrade already, so to not
> have to refactor the build of modules now and then again shortly.
>
> As a reminder, we want Infinispan to be able to provide a "Lucene
> Directory Provider" for 3 versions of Hibernate Search:
>  A) the version which is included in latest WildFly stable release
>  B) the version which Infinispan Query is using
>  C) the latest stable version
>
> It is of course possible - even likely - that some of these versions
> happen to be the same for a particular release train; that's nice
> however please remember that while they might be *coincidentally* the
> same, that's no good reason to remove the capability of the build
> system to support these different versions when they happen to
> diverge. That's why the properties which mark each of these versions
> was strictly separate - and more crucially was not "redundant".
>
> In the specific case, I was looking for the properties I needed to
> change to make sure we could support latest Hibernate Search release
> but they were gone, my guess is this was caused by the fact that
> recently version[B] and version[C] happened to match so this confused.
>
> At this stage I don't think it's worth it to try find and identify all
> changes which should be reverted, as with the upcoming upgrade to
> Hibernate Search 5.9.0.Final several changes in how we build modules
> would be needed, so I'll send a PR to upgrade to HS 5.9.0.Final
> already.
>
> Risks and implications?
>
> HS 5.9 is mostly the same as 5.8, the main difference for end users is
> the integration with a different version of Hibernate ORM now
> supporting JPA 2.2 - but this isn't relevant for Infinispan so I
> consider the upgrade overall a low risk.
> The other relevant difference is that we no longer publish a zip of
> the modules for WildFly - we publish instead a set of rather fine
> grained feature packs. This is good news for Infinispan as we can
> better cherry pick which components you actually want, but it means
> the build needs to be adapted now to deal with these feature packs.
>
> In my PR for ISPN-8779 I'll use the Provisioning plugin for Maven to
> materialize the modules that Infinispan needs, so that for now they
> can be re-packaged into the zip files - so to not have any impact on
> Infinispan users. I hope in a second stage you'll all see the benefits
> of distributing feature packs so we'll be able to simplify some
> things.
>
> Feature packs normally have a notion of dependency to other feature
> packs, but to keep the adaptation into "old style fat zip" I'll
> reconfigure the provisioning task to disable transitive dependencies;
> the drawback is that I'll have to declare the version of each feature
> pack we need explicitly in the parent pom: we'll be able to avoid the
> need to match the dependant versions when Infinispan also will produce
> feature packs rather than the fat zip.
>
> N.B. this PR #5766 doesn't address the fact that the build doesn't
> differentiate between the above cases B and C, but since B and C would
> just happen to be the same version again in Infinispan 9.2 the issue
> is no longer urgent, so ISPN-8780 could be postponed to 9.3+ and it
> might be much easier after migrating some Infinispan modules to
> feature packs as well.
>
> Thanks,
> Sanne
>
>
> On 7 February 2018 at 16:59, Sanne Grinovero <[hidden email]> wrote:
> > Hi all,
> >
> > I was going to give ISPN-8779 a shot but I'm finding a mess.
> >
> > the root pom contains these twice (and inconsistent!):
> >
> > <version.hibernate.search>5.8.1.Final</version.hibernate.search>
> > [...]
> > <version.hibernate.search>5.8.0.Final</version.hibernate.search>
> >
> > the BOM cointains a copy of `version.hibernate.search` as well.
> >
> > I don't mind deleting duplicate properties, but we used to have
> > clearly separate properties for different purposes, and this
> > separation is essential.
> >
> > I've mentioned this multiple times when reviewing PRs which would get
> > my attention, but I didn't see these changes - certainly didn't expect
> > you all to forget the special purpose of these modules.
> > It's quite messy now and I'm honestly lost myself at how I could revert it.
> >
> > In particular this module is broken now as it's targeting the wrong slot:
> >  -
> >  https://github.com/infinispan/infinispan/blob/master/wildfly-modules/src/main/resources/org/infinispan/for-hibernatesearch-wildfly/main/module.xml#L27
> >
> > Clearly it's not consistent with the comment I've put on the module
> > descriptor.
> > I don't see that module being included in the released modules either,
> > and clearly the integration tests didn't catch it because they have
> > been patched to use the wrong modules too :(
> >
> > Other essential integration tests which I had put in place to make
> > sure they'd get your attention in case someone had such an idea.. have
> > been deleted.
> >
> > Opening ISPN-8780, I would consider this a release blocker.
> >
> > Thanks,
> > Sanne
> >
> > See also:
> >  -
> >  https://github.com/infinispan/infinispan/blob/master/integrationtests/as-lucene-directory/READ.ME
> _______________________________________________
> 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
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Hibernate Search integration modules for WildFly have been broken

Sanne Grinovero-3
Hi Ryan,

no problem at all. I can see how it's confusing, and open to
suggestions to make this clearer. Also I don't blame who's working to
make progress but I'll blame the reviewers, they should know by know
about these modules :P

Regarding the Search upgrade, that's up to Adrian, Gustavo and
Tristan, but as I mentioned the version upgrade was classified as a
minor mostly because of the ORM integration changes, which won't
affect Infinispan so not very disruptive at all - except in how those
modules have to be built of course.

Thanks,
Sanne




On 19 February 2018 at 10:29, Ryan Emerson <[hidden email]> wrote:

> Hi Sanne,
>
> Thanks for the input and the proposed solution. I am not familiar
> with Hibernate search and our integrations, so I will leave others to
> comment on the version upgrade. I just wanted to let you know that the
> transition to feature-packs is on the roadmap for 9.3 as part of our
> ongoing server refactoring efforts. Furthermore, I believe the breaking
> of the wildfly-modules was caused by my work on the bom and parent pom
> refactoring, so apologies for the inconvenience, I owe you a beer!
>
> Cheers
> Ryan
>
> ----- Original Message -----
>> Hi all,
>>
>> spent friday and a good deal of the weekend exploring some options;
>> finally I have a proposal.
>>  - https://github.com/infinispan/infinispan/pull/5766
>>
>> But let me give some background:
>>
>> I was originally planning to upgrade Infinispan to Hibernate Search
>> 5.9.0.Final only in Infinispan 9.3, as while the code itself didn't
>> change much, I was expecting quite some work in the area of the build
>> of WildFly modules.
>>
>> However since the module structure in the current master is lacking
>> [ISPN-8780] I'm proposing now to do this upgrade already, so to not
>> have to refactor the build of modules now and then again shortly.
>>
>> As a reminder, we want Infinispan to be able to provide a "Lucene
>> Directory Provider" for 3 versions of Hibernate Search:
>>  A) the version which is included in latest WildFly stable release
>>  B) the version which Infinispan Query is using
>>  C) the latest stable version
>>
>> It is of course possible - even likely - that some of these versions
>> happen to be the same for a particular release train; that's nice
>> however please remember that while they might be *coincidentally* the
>> same, that's no good reason to remove the capability of the build
>> system to support these different versions when they happen to
>> diverge. That's why the properties which mark each of these versions
>> was strictly separate - and more crucially was not "redundant".
>>
>> In the specific case, I was looking for the properties I needed to
>> change to make sure we could support latest Hibernate Search release
>> but they were gone, my guess is this was caused by the fact that
>> recently version[B] and version[C] happened to match so this confused.
>>
>> At this stage I don't think it's worth it to try find and identify all
>> changes which should be reverted, as with the upcoming upgrade to
>> Hibernate Search 5.9.0.Final several changes in how we build modules
>> would be needed, so I'll send a PR to upgrade to HS 5.9.0.Final
>> already.
>>
>> Risks and implications?
>>
>> HS 5.9 is mostly the same as 5.8, the main difference for end users is
>> the integration with a different version of Hibernate ORM now
>> supporting JPA 2.2 - but this isn't relevant for Infinispan so I
>> consider the upgrade overall a low risk.
>> The other relevant difference is that we no longer publish a zip of
>> the modules for WildFly - we publish instead a set of rather fine
>> grained feature packs. This is good news for Infinispan as we can
>> better cherry pick which components you actually want, but it means
>> the build needs to be adapted now to deal with these feature packs.
>>
>> In my PR for ISPN-8779 I'll use the Provisioning plugin for Maven to
>> materialize the modules that Infinispan needs, so that for now they
>> can be re-packaged into the zip files - so to not have any impact on
>> Infinispan users. I hope in a second stage you'll all see the benefits
>> of distributing feature packs so we'll be able to simplify some
>> things.
>>
>> Feature packs normally have a notion of dependency to other feature
>> packs, but to keep the adaptation into "old style fat zip" I'll
>> reconfigure the provisioning task to disable transitive dependencies;
>> the drawback is that I'll have to declare the version of each feature
>> pack we need explicitly in the parent pom: we'll be able to avoid the
>> need to match the dependant versions when Infinispan also will produce
>> feature packs rather than the fat zip.
>>
>> N.B. this PR #5766 doesn't address the fact that the build doesn't
>> differentiate between the above cases B and C, but since B and C would
>> just happen to be the same version again in Infinispan 9.2 the issue
>> is no longer urgent, so ISPN-8780 could be postponed to 9.3+ and it
>> might be much easier after migrating some Infinispan modules to
>> feature packs as well.
>>
>> Thanks,
>> Sanne
>>
>>
>> On 7 February 2018 at 16:59, Sanne Grinovero <[hidden email]> wrote:
>> > Hi all,
>> >
>> > I was going to give ISPN-8779 a shot but I'm finding a mess.
>> >
>> > the root pom contains these twice (and inconsistent!):
>> >
>> > <version.hibernate.search>5.8.1.Final</version.hibernate.search>
>> > [...]
>> > <version.hibernate.search>5.8.0.Final</version.hibernate.search>
>> >
>> > the BOM cointains a copy of `version.hibernate.search` as well.
>> >
>> > I don't mind deleting duplicate properties, but we used to have
>> > clearly separate properties for different purposes, and this
>> > separation is essential.
>> >
>> > I've mentioned this multiple times when reviewing PRs which would get
>> > my attention, but I didn't see these changes - certainly didn't expect
>> > you all to forget the special purpose of these modules.
>> > It's quite messy now and I'm honestly lost myself at how I could revert it.
>> >
>> > In particular this module is broken now as it's targeting the wrong slot:
>> >  -
>> >  https://github.com/infinispan/infinispan/blob/master/wildfly-modules/src/main/resources/org/infinispan/for-hibernatesearch-wildfly/main/module.xml#L27
>> >
>> > Clearly it's not consistent with the comment I've put on the module
>> > descriptor.
>> > I don't see that module being included in the released modules either,
>> > and clearly the integration tests didn't catch it because they have
>> > been patched to use the wrong modules too :(
>> >
>> > Other essential integration tests which I had put in place to make
>> > sure they'd get your attention in case someone had such an idea.. have
>> > been deleted.
>> >
>> > Opening ISPN-8780, I would consider this a release blocker.
>> >
>> > Thanks,
>> > Sanne
>> >
>> > See also:
>> >  -
>> >  https://github.com/infinispan/infinispan/blob/master/integrationtests/as-lucene-directory/READ.ME
>> _______________________________________________
>> 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
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev