Hi Amin,
I couldn't find your patch, could you attach it to the issue?
I don't think this mail list accepts attachments.
Cheers,
Sanne
2010/3/25 Amin Mohammed-Coleman <ami...@gmail.com>:
> Hi
>
> I have created a patch based on Emmanuel's thoughts. I haven't applied the
> usage of the ErrorHandler yet as I wanted to get an idea on whether it was
> the right path. I have mentioned this before but I'm not sure how to capture
> the operation at fault followed by subsequent failures. I'd appreciate it if
> someone could give me a pointer.
>
> I think I spoke to Sanne about this and the approach was to capture each of
> the exceptions and log it (or configured by the user), but the error context
> doesn't fit with this approach.
>
>
>
> Thanks
> Amin
>
>
>
>
>
>
> On 22 Mar 2010, at 17:43, Emmanuel Bernard 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
>
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
