HSEARCH-1225 #370

Merged
merged 2 commits into from Jan 9, 2013

Projects

None yet

3 participants

@hferentschik hferentschik commented on the diff Jan 9, 2013
...ntation/src/main/docbook/en-US/modules/batchindex.xml
@@ -66,6 +66,12 @@ tx.commit(); //index only updated at commit time</programlisting>
types, the recommended approach is to use a
<classname>MassIndexer</classname>: see <xref
linkend="search-batchindex-massindexer"/> for more details.</para>
+
+ <para>The method <methodname>FullTextSession.index(T
@hferentschik
hferentschik Jan 9, 2013 Member

hmm, that does not seem to be related to HSEARCH-1225, right?
Btw, is this supposed to just document the current behavior or do we really want this to work like this in the long term as well. Didn't we have a discussion around this. Personally I find this behavior inconsistent.

@hferentschik
Member

HSEARCH-1225 is for sure the wrong issue key for that. Not sure which issue you relates to this doc changes

@emmanuelbernard emmanuelbernard merged commit 77c0cea into hibernate:master Jan 9, 2013
@hferentschik
Member

Sorry, taking my wrong issue key comment back. I looked at the wrong browser tab :-(
Still think we have an inconsistency in the how interception works, but I guess this is just documenting the status quo.

@Sanne
Member
Sanne commented Jan 10, 2013

Sorry, taking my wrong issue key comment back. I looked at the wrong browser tab :-(

That's good news, or I would be annoyed it was merged :-)

Still think we have an inconsistency in the how interception works, but I guess this is just documenting the status quo.

I remember we had disagreements on this, but I don't remember all the argumentation; we can resume the debate after the release if you want.
I still feel this is correct but I'll of course fight my feeling back for the sake of good reasoning; We could change the Interceptor API to add explicit methods to "have a say" about those indexing methods specifically - so that it can react differently to events vs. API call .. all things we can still change without breaking the current (experimental) feature.

Today, the IndexingInterceptor is applied to automatically handled events, and this only. This is important to document.
I don't see why it would need to be applied to something which is not automatic, as that is under the developer's control.

@hferentschik
Member

That's good news, or I would be annoyed it was merged :-)

Yeah, I double checked after @emmanuelbernard so ruthlessly merged ;-)

I remember we had disagreements on this, but I don't remember all the argumentation; we can resume the debate after the release if you want.

+1

@emmanuelbernard
Member

AFAIR we needed the manual index/purge calls to be unfiltered to cover some use cases.

On 10 janv. 2013, at 01:43, Sanne Grinovero notifications@github.com wrote:

Sorry, taking my wrong issue key comment back. I looked at the wrong browser tab :-(

That's good news, or I would be annoyed it was merged :-)

Still think we have an inconsistency in the how interception works, but I guess this is just documenting the status quo.

I remember we had disagreements on this, but I don't remember all the argumentation; we can resume the debate after the release if you want.
I still feel this is correct but I'll of course fight my feeling back for the sake of good reasoning; We could change the Interceptor API to add explicit methods to "have a say" about those indexing methods specifically - so that it can react differently to events vs. API call .. all things we can still change without breaking the current (experimental) feature.

Today, the IndexingInterceptor is applied to automatically handled events, and this only. This is important to document.
I don't see why it would need to be applied to something which is not automatic, as that is under the developer's control.


Reply to this email directly or view it on GitHub.

@hferentschik
Member

AFAIR we needed the manual index/purge calls to be unfiltered to cover some use cases

Right, but you could also have some explicit way to enable/disable the interceptor application via FullTextSession w/ the default being enabled.

@Sanne
Member
Sanne commented Jan 11, 2013

I think the way to go is to add methods to the Interceptor interface so that the user can explicitly define what he wants to happen for index/purge; added benefit is that since he has to implement it, no doubt he will know what to expect.

Not at this dev round of course.

@emmanuelbernard
Member

Add a comment in the doc asking people to contact us if they have this need.
Otherwise, I'd rather not offer the option. My gut feeling is that nobody will need it.

On 11 janv. 2013, at 12:41, Hardy Ferentschik notifications@github.com wrote:

AFAIR we needed the manual index/purge calls to be unfiltered to cover some use cases

Right, but you could also have some explicit way to enable/disable the interceptor application via FullTextSession w/ the default being enabled.


Reply to this email directly or view it on GitHub.

@emmanuelbernard
Member

Not 100% sure that covers all possible use case but again I'd rather see people come with real use cases we can discuss before a move.

On 11 janv. 2013, at 12:43, Sanne Grinovero notifications@github.com wrote:

I think the way to go is to add methods to the Interceptor interface so that the user can explicitly define what he wants to happen for index/purge; added benefit is that since he has to implement it, no doubt he will know what to expect.

Not at this dev round of course.


Reply to this email directly or view it on GitHub.

@Sanne
Member
Sanne commented Jan 11, 2013

nice idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment