You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2016/05/03 00:32:02 UTC
Review Request 46914: GEODE-11: Fixing a class cast exception when
LuceneFunction has an error
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46914/
-----------------------------------------------------------
Review request for geode, anilkumar gingade, Barry Oglesby, and nabarun nag.
Repository: geode
Description
-------
LuceneFunction was using sendException to return exceptions to caller.
But the behavior of sendException is actually to pass the exception to
the addResult method, which is not what we want in this case.
Adding an integration test of the same. Changing LuceneFunctionJUnitTest
to use mockito and changing the excepectations of what LuceneFunction
will do after an exception.
Diffs
-----
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java 199b698c295e901cbd5450c263a6301f6db8970c
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesCollectorManager.java b19e1041d830363383c1bfa00bed6f7ac8a4b057
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationJUnitTest.java PRE-CREATION
geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 750ec0f6ba2a07e905676900003c24b06647f8ee
Diff: https://reviews.apache.org/r/46914/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 46914: GEODE-11: Fixing a class cast exception when
LuceneFunction has an error
Posted by Dan Smith <ds...@pivotal.io>.
> On May 2, 2016, 11:55 p.m., nabarun nag wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java, line 87
> > <https://reviews.apache.org/r/46914/diff/1/?file=1369149#file1369149line87>
> >
> > Dan just for me to learn, why is a FunctionException being thrown rather than a QueryException
QueryException and IOException are checked exceptions. Since Function.execute doesn't have any checked Exceptions in the signature, we have to wrap these exceptions in order to throw them.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46914/#review131420
-----------------------------------------------------------
On May 2, 2016, 10:32 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46914/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 10:32 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, and nabarun nag.
>
>
> Repository: geode
>
>
> Description
> -------
>
> LuceneFunction was using sendException to return exceptions to caller.
> But the behavior of sendException is actually to pass the exception to
> the addResult method, which is not what we want in this case.
>
> Adding an integration test of the same. Changing LuceneFunctionJUnitTest
> to use mockito and changing the excepectations of what LuceneFunction
> will do after an exception.
>
>
> Diffs
> -----
>
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java 199b698c295e901cbd5450c263a6301f6db8970c
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesCollectorManager.java b19e1041d830363383c1bfa00bed6f7ac8a4b057
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 750ec0f6ba2a07e905676900003c24b06647f8ee
>
> Diff: https://reviews.apache.org/r/46914/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 46914: GEODE-11: Fixing a class cast exception when
LuceneFunction has an error
Posted by nabarun nag <nn...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46914/#review131420
-----------------------------------------------------------
Dan, these comments are just for my understanding[no actual issues with the code.]
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java (line 85)
<https://reviews.apache.org/r/46914/#comment195394>
Dan just for me to learn, why is a FunctionException being thrown rather than a QueryException
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java (line 111)
<https://reviews.apache.org/r/46914/#comment195402>
Same here why rather doing an isInstanceOf call and throwing the corresponding exception
- nabarun nag
On May 2, 2016, 10:32 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46914/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 10:32 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, and nabarun nag.
>
>
> Repository: geode
>
>
> Description
> -------
>
> LuceneFunction was using sendException to return exceptions to caller.
> But the behavior of sendException is actually to pass the exception to
> the addResult method, which is not what we want in this case.
>
> Adding an integration test of the same. Changing LuceneFunctionJUnitTest
> to use mockito and changing the excepectations of what LuceneFunction
> will do after an exception.
>
>
> Diffs
> -----
>
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java 199b698c295e901cbd5450c263a6301f6db8970c
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesCollectorManager.java b19e1041d830363383c1bfa00bed6f7ac8a4b057
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 750ec0f6ba2a07e905676900003c24b06647f8ee
>
> Diff: https://reviews.apache.org/r/46914/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 46914: GEODE-11: Fixing a class cast exception when
LuceneFunction has an error
Posted by Barry Oglesby <bo...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46914/#review131410
-----------------------------------------------------------
The source changes look good. I'm not sure I'm the best guy to review the test changes.
- Barry Oglesby
On May 2, 2016, 10:32 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46914/
> -----------------------------------------------------------
>
> (Updated May 2, 2016, 10:32 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, and nabarun nag.
>
>
> Repository: geode
>
>
> Description
> -------
>
> LuceneFunction was using sendException to return exceptions to caller.
> But the behavior of sendException is actually to pass the exception to
> the addResult method, which is not what we want in this case.
>
> Adding an integration test of the same. Changing LuceneFunctionJUnitTest
> to use mockito and changing the excepectations of what LuceneFunction
> will do after an exception.
>
>
> Diffs
> -----
>
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunction.java 199b698c295e901cbd5450c263a6301f6db8970c
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/distributed/TopEntriesCollectorManager.java b19e1041d830363383c1bfa00bed6f7ac8a4b057
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneQueriesIntegrationJUnitTest.java PRE-CREATION
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/LuceneFunctionJUnitTest.java 750ec0f6ba2a07e905676900003c24b06647f8ee
>
> Diff: https://reviews.apache.org/r/46914/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>