Hi Emmanuel
Thanks for your input. I've just had the eureka moment which I was
struggling with. Please give me a day or two and create a patch that
conforms to your thoughts
Thanks for your patience!
Cheers
Amin
Sent from my iPhone
On 22 Mar 2010, at 17:43, Emmanuel Bernard <emman...@hibernate.org>
wrote:
> Hello,
> Sorry, I've been a bit out of this particular problem for a long
> time, so if I ask already answers questions forgive me.
>
> Thanks for all the work, sorry it take so long on our part for each
> review :(
>
> Here are some comments on the patch:
> - your code rules don't seem to match Hibernate's in particular you
> use import com.acme.*; which we don't. It makes the patch less
> readable for us as many areas are changes wo need.
> - getSyncBackendExceptionHandler / getAsyncBackendExceptionHandler
> do a lot of init. Are they called several times? I'd rather see them
> named build* instead of get*
> - don't use log.warn when creating the exception handler. Either do
> nothing or raise an exception. In this case I'd favor raising a
> SearchException
> - this has been discussed I think but do we need an async and a sync
> exception handler? Or do we need an exception handler? What's the
> rational for the split? If a split is necessary, what's the rational
> for two different APIs?
> - I don't like classes named Default*, they should have a meaningful
> name and purpose somehow: LogExceptionHandler,
> RethrowExceptionHandler etc
> - Why exactly do we not return anything during the sync and return
> Thread.UncaughtExceptionHandler during the async? Couldn't we write
> adaptors to hide the complexity to the user if some transformation
> is needed?
> - we should pass more data to the exception. I imagine in many cases
> we know what entity is being indexed (type and id), if we create or
> delete the index. There are useful info to rerun the indexing
> process or analyze what's going on. Can we imagine an API for that?
> Something like:
>
> class ErrorHandler {
> void handle(ErrorContext context);
> }
>
> interface ErrorContext {
> //TODO list or set?
> Set<FailingOperation> getFailingOperations();
> FailingOperation getOperationAtFault();
> //TODO should it be Throwable or something more specific like
> SearchException?
> Throwable getThrownException();
> }
>
> intarface FailingOperation {
> Object getId(); //is it Serializable?
> Class<?> getEntityClass();
> Operation getOperation();
> }
>
> WDYT?
>
> On 19 mars 2010, at 20:55, Amin Mohammed-Coleman wrote:
>
>>
>> Hi
>>
>> Please find updated patch with the removal of the enum singleton.
>>
>> Thanks
>> Amin<HibernateSearch421-19032010.patch>
>> On 17 Mar 2010, at 12:58, Sanne Grinovero wrote:
>>
>>> Hi Amin,
>>> thanks for the update, see some thoughts:
>>>
>>> 2010/3/16 Amin Mohammed-Coleman <ami...@gmail.com>:
>>>> Hi folks,
>>>>
>>>> I've removed the enum singleton and created a class
>>>> (BackendExceptionHandler) which has 2 methods:
>>>>
>>>>
>>>> public Thread.UncaughtExceptionHandler getUncaughtExceptionHandler
>>>> (SearchConfiguration configuration);
>>>>
>>>> public boolean logException(SearchConfiguration configuration)
>>>
>>> Care to explain how I should use them? Are we not going to have a
>>> common interface? In that case does it make sense to have a method
>>> named "logException", which would imply a logging implementation?
>>>
>>>>
>>>> The issue that I'm looking at is getting the search configuration
>>>> to the methods. In order to get the SearchConfiguration to the
>>>> methods defined in BackendExceptionHandler, I have to thread
>>>> SearchFactoryImplementor.
>>>>
>>>> Using this approach I will have to define a method to get the
>>>> search configuration from the SearchFactoryImplementor. I'm
>>>> guess this isn't the best approach as this requires a significant
>>>> change. I don't know what peoples thoughts are on this. I'm
>>>> looking to set the BackendExceptionHandler up when the
>>>> SearchFactory is created and then use it. Is there any currently
>>>> approach that does something similar?
>>>
>>> Ah there's a subtle problem with that, which is that we can't hold a
>>> reference in Search to a Configuration at runtime: use it at
>>> SearchFactory creation, extract all what you need, but then you have
>>> to clear references or we're going to have a memory leak. [1]
>>> The solution is, as we do with all components, to start them during
>>> SearchFactory initialization and then expose a getter to initialized
>>> service.
>>>
>>> [1] http://opensource.atlassian.com/projects/hibernate/browse/HSEARCH-314
>>>
>>> Cheers and thanks for the complex work,
>>> Sanne
>>>
>>>>
>>>>
>>>> Thanks for your patience with this one.
>>>>
>>>> Cheers
>>>> Amin
>>>>
>>>>
>>>>
>>>>
>>>> On 9 Mar 2010, at 13:00, Amin Mohammed-Coleman wrote:
>>>>
>>>>> Hi Sanne
>>>>>
>>>>> You are right and Im not happy with the enum class. I wanted to
>>>>> have a single configuration that was available on the creation
>>>>> of the search factory and re use when required. I'll take a
>>>>> look at changing that with a better solution.
>>>>>
>>>>> Cheers
>>>>>
>>>>> Amin
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On 9 Mar 2010, at 12:47, Sanne Grinovero <sanne.grinov...@gmail.com
>>>>> > wrote:
>>>>>
>>>>>> yes that is it;
>>>>>> There has been some talking about other strategies as well on
>>>>>> previous
>>>>>> mails, like jms, but that lead to nowhere so yes I'm suggesting
>>>>>> now to
>>>>>> forget about other default implementations at the moment and
>>>>>> proceed
>>>>>> as you just said.
>>>>>>
>>>>>> 2010/3/9 Emmanuel Bernard <emman...@hibernate.org>:
>>>>>>> I thought the goal was to have something pluggable with two
>>>>>>> default impls (exception and log). If that's what you are
>>>>>>> describing I am ok, if not, then I don't understand ;)
>>>>>>>
>>>>>>> On 9 mars 2010, at 12:08, Sanne Grinovero wrote:
>>>>>>>
>>>>>>>> Emmanuel are you ok with this if we either log or rethrow the
>>>>>>>> exception back to application code? jms et al looks complex
>>>>>>>> and I
>>>>>>>> don't see the benefit.
>>>>>>>
>>>>>>>
>>>>
>>>>
>>
>
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
