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

HSEARCH-1268 Make it possible to plug in a custom MassIndexer implementation #380

Closed
wants to merge 4 commits into from

Conversation

DavideD
Copy link
Member

@DavideD DavideD commented Feb 1, 2013

https://hibernate.onjira.com/browse/HSEARCH-1268

I have a couple of question. I will ask as inline comments

@DavideD
Copy link
Member Author

DavideD commented Feb 1, 2013

I don't think I can leave on inline comment on this one because it's about file I didn't change, anyway:

ClassLoaderHelper#classForName is using the contextClassloader before using the one passed as a parameter. Shouldn't be the opposite?

@Sanne
Copy link
Member

Sanne commented Feb 2, 2013

ClassLoaderHelper#classForName is using the contextClassloader before using the one passed as a parameter. Shouldn't be the opposite?

Order is not important, it only has a performance impact as it might try the correct one first, avoiding one reource access.
We expect most things to be found on the contextClassLoader as that's where the user resources are (like lists of stopwords, custom analyzers, any other extension)

}

private MassIndexerFactory createFactory(String factoryClassName) {
return ClassLoaderHelper.instanceFromName( MassIndexerFactory.class, factoryClassName, getClass()
Copy link
Member

Choose a reason for hiding this comment

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

did you check if ORM isn't providing you the classloader as a Service?

 getClass().getClassLoader()

If we still use this code, then the fix in the previous commit isn't as useful as I thought it would be.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't check.

I will take a look at it

@DavideD
Copy link
Member Author

DavideD commented Feb 2, 2013

ClassLoaderHelper#classForName is using the contextClassloader before using the one passed as a parameter. Shouldn't be the opposite?

Order is not important, it only has a performance impact as it might try the correct one first, avoiding one reource access.
We expect most things to be found on the contextClassLoader as that's where the user resources are (like lists of stopwords, custom analyzers, any other extension)

I'm asking because It's seems a bit awkward that I pass the classloader that I want to use and then another one is used. By the way, I've noticed this writing some unit tests.

@DavideD
Copy link
Member Author

DavideD commented Feb 4, 2013

Added some documentation and javadoc

@DavideD
Copy link
Member Author

DavideD commented Feb 5, 2013

Rebased. All the changes should be applied now.

@DavideD
Copy link
Member Author

DavideD commented Feb 5, 2013

Changed a comment

@Sanne
Copy link
Member

Sanne commented Feb 5, 2013

merged!

@Sanne Sanne closed this Feb 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants