You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jared Stewart <js...@pivotal.io> on 2017/08/18 21:55:53 UTC
Review Request 61758: GEODE-3471: Identify NPE in MBeanProxyFactory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61758/
-----------------------------------------------------------
Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
GEODE-3471: Identify NPE in MBeanProxyFactory
Diffs
-----
geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java 0cce0be22323cf47dd6f90691bb068b3aaa77372
Diff: https://reviews.apache.org/r/61758/diff/1/
Testing
-------
Precheckin running
Thanks,
Jared Stewart
Re: Review Request 61758: GEODE-3471: Identify NPE in
MBeanProxyFactory
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61758/#review183397
-----------------------------------------------------------
Ship it!
Ship It!
- Patrick Rhomberg
On Aug. 18, 2017, 9:55 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61758/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 9:55 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3471: Identify NPE in MBeanProxyFactory
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java 0cce0be22323cf47dd6f90691bb068b3aaa77372
>
>
> Diff: https://reviews.apache.org/r/61758/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 61758: GEODE-3471: Identify NPE in
MBeanProxyFactory
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61758/#review183399
-----------------------------------------------------------
Ship it!
Ship It!
- Kirk Lund
On Aug. 18, 2017, 9:55 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61758/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 9:55 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3471: Identify NPE in MBeanProxyFactory
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java 0cce0be22323cf47dd6f90691bb068b3aaa77372
>
>
> Diff: https://reviews.apache.org/r/61758/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 61758: GEODE-3471: Identify NPE in
MBeanProxyFactory
Posted by Jared Stewart <js...@pivotal.io>.
> On Aug. 21, 2017, 5:55 p.m., Patrick Rhomberg wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java
> > Lines 83-84 (original), 81-85 (patched)
> > <https://reviews.apache.org/r/61758/diff/1/?file=1800306#file1800306line83>
> >
> > I might be missing context, but this just a spike to see which of these methods is returning `null`, yeah?
> >
> > Is receiving `null` in this context indicative of failure, or should there be some null checks and handling? Should we formally check against null and throw instead of allowing an NPE to bubble up? Would that be a more meaningful error than an NPE?
This change is just to help see what's null. Once we know, we'll be able to decide how to properly deal with a null value.
- Jared
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61758/#review183354
-----------------------------------------------------------
On Aug. 18, 2017, 9:55 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61758/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 9:55 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3471: Identify NPE in MBeanProxyFactory
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java 0cce0be22323cf47dd6f90691bb068b3aaa77372
>
>
> Diff: https://reviews.apache.org/r/61758/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>
Re: Review Request 61758: GEODE-3471: Identify NPE in
MBeanProxyFactory
Posted by Patrick Rhomberg <pr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61758/#review183354
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java
Lines 83-84 (original), 81-85 (patched)
<https://reviews.apache.org/r/61758/#comment259386>
I might be missing context, but this just a spike to see which of these methods is returning `null`, yeah?
Is receiving `null` in this context indicative of failure, or should there be some null checks and handling? Should we formally check against null and throw instead of allowing an NPE to bubble up? Would that be a more meaningful error than an NPE?
- Patrick Rhomberg
On Aug. 18, 2017, 9:55 p.m., Jared Stewart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61758/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 9:55 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-3471: Identify NPE in MBeanProxyFactory
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java 0cce0be22323cf47dd6f90691bb068b3aaa77372
>
>
> Diff: https://reviews.apache.org/r/61758/diff/1/
>
>
> Testing
> -------
>
> Precheckin running
>
>
> Thanks,
>
> Jared Stewart
>
>