You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2015/02/12 15:38:56 UTC
Review Request 30932: Current Alerts Do Not Reflect the Name Change
of an Alert Definition
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30932/
-----------------------------------------------------------
Review request for Ambari, Nate Cole and Tom Beerbower.
Bugs: AMBARI-9597
https://issues.apache.org/jira/browse/AMBARI-9597
Repository: ambari
Description
-------
When changing the name of an Alert Definition, the current alerts associated with that definition don't reflect the name change. We want current alerts and alerts movingn forward to reflect the name change, but not the past alerts.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java d57bbd0
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 10b93aa
ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionChangedEvent.java ad063a3
ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertLifecycleListener.java f21d800
ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java a5a3614
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 34216e7
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java 6fe825f
ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java 897109c
ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java 02c80ed
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java d588069
ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertEventPublisherTest.java 6331516
Diff: https://reviews.apache.org/r/30932/diff/
Testing
-------
Manual verification of the issue and the fix, including that history does not change. Added new tests.
Thanks,
Jonathan Hurley
Re: Review Request 30932: Current Alerts Do Not Reflect the Name
Change of an Alert Definition
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30932/#review72167
-----------------------------------------------------------
Ship it!
Looks good.
- Tom Beerbower
On Feb. 12, 2015, 2:38 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30932/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2015, 2:38 p.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-9597
> https://issues.apache.org/jira/browse/AMBARI-9597
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When changing the name of an Alert Definition, the current alerts associated with that definition don't reflect the name change. We want current alerts and alerts movingn forward to reflect the name change, but not the past alerts.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java d57bbd0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 10b93aa
> ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionChangedEvent.java ad063a3
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertLifecycleListener.java f21d800
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java a5a3614
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 34216e7
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java 6fe825f
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java 897109c
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java 02c80ed
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java d588069
> ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertEventPublisherTest.java 6331516
>
> Diff: https://reviews.apache.org/r/30932/diff/
>
>
> Testing
> -------
>
> Manual verification of the issue and the fix, including that history does not change. Added new tests.
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 30932: Current Alerts Do Not Reflect the Name
Change of an Alert Definition
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Feb. 12, 2015, 10:04 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java, lines 216-218
> > <https://reviews.apache.org/r/30932/diff/1/?file=861752#file861752line216>
> >
> > Doesn't @Override imply this?
Thanks for the review!
There used to be a bug in Eclipse where the Javadoc wouldn't be inherited. I started doing this b/c of that. Now that Eclipse is fixed, I'm just so used to it. I believe it helps in the visual differentiation of the methods in the files making it easier to scan quickly. Also it helps prevent the issue where someone wants to add a comment to a method and forgets the {@inheritDoc}, causing the base documentation to be lost.
It's one of those things that I guess is chalked up to "Developer Preference". You know, like, "I'll stop using Whitesmiths when you pry the keyboard from my cold, dead hands".
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30932/#review72163
-----------------------------------------------------------
On Feb. 12, 2015, 9:38 a.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30932/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2015, 9:38 a.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-9597
> https://issues.apache.org/jira/browse/AMBARI-9597
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When changing the name of an Alert Definition, the current alerts associated with that definition don't reflect the name change. We want current alerts and alerts movingn forward to reflect the name change, but not the past alerts.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java d57bbd0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 10b93aa
> ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionChangedEvent.java ad063a3
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertLifecycleListener.java f21d800
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java a5a3614
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 34216e7
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java 6fe825f
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java 897109c
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java 02c80ed
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java d588069
> ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertEventPublisherTest.java 6331516
>
> Diff: https://reviews.apache.org/r/30932/diff/
>
>
> Testing
> -------
>
> Manual verification of the issue and the fix, including that history does not change. Added new tests.
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 30932: Current Alerts Do Not Reflect the Name
Change of an Alert Definition
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30932/#review72163
-----------------------------------------------------------
Ship it!
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java
<https://reviews.apache.org/r/30932/#comment118167>
Doesn't @Override imply this?
- Nate Cole
On Feb. 12, 2015, 9:38 a.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30932/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2015, 9:38 a.m.)
>
>
> Review request for Ambari, Nate Cole and Tom Beerbower.
>
>
> Bugs: AMBARI-9597
> https://issues.apache.org/jira/browse/AMBARI-9597
>
>
> Repository: ambari
>
>
> Description
> -------
>
> When changing the name of an Alert Definition, the current alerts associated with that definition don't reflect the name change. We want current alerts and alerts movingn forward to reflect the name change, but not the past alerts.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java d57bbd0
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 10b93aa
> ambari-server/src/main/java/org/apache/ambari/server/events/AlertDefinitionChangedEvent.java ad063a3
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertLifecycleListener.java f21d800
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertReceivedListener.java a5a3614
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertsDAO.java 34216e7
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java 6fe825f
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java 897109c
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java 02c80ed
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/AlertsDAOTest.java d588069
> ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertEventPublisherTest.java 6331516
>
> Diff: https://reviews.apache.org/r/30932/diff/
>
>
> Testing
> -------
>
> Manual verification of the issue and the fix, including that history does not change. Added new tests.
>
>
> Thanks,
>
> Jonathan Hurley
>
>