> On June 12, 2016, 7:06 a.m., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/base_alert.py, lines 
> > 410-411
> > <https://reviews.apache.org/r/48589/diff/1/?file=1415725#file1415725line410>
> >
> >     This still seems like a warning which we want. It means that there was 
> > a name service defined, but no alias properties. If a name service is 
> > defined, then shouldn't there always be valid aliases?
> 
> Miklos Gergely wrote:
>     The uri block of some alerts contains a high_availability sub block, 
> which may define a nameservice. If HA is not enabled than the defined 
> property is not present. I don't see any logic to tell earlier whether or not 
> HA is enabled, so the fact that the property the HA nameservice references is 
> missing is a good indicator that it's not. If there is anoyther way to decide 
> if HA is enabled let me know about it, that logic could be used at 
> base_alert:342 to decide whether or not to enter into 
> _get_uri_from_ha_structure at all. Currently the logic there is to check if 
> ha_nameservice or ha_alias_key is defined, but the comment above it tells 
> that in this case it should "try" to get these properties, i.e. it is still 
> not sure that they are there. Following this logic their absence is not a 
> cause for warning.
> 
> Jonathan Hurley wrote:
>     There is really no way to determine if HA is "enabled". That's why we use 
> a combinaton of the nameservice and the alias. If the nameservice is present 
> in the configs, then I think the aliases should be as well. I think the logic 
> as it is today is a little wrong. It should be more like:
>     
>     ```
>     if ha_nameservice is None:
>       return None
>     
>     if ha_nameservice is not None and ha_alias_key is None:
>       return None
>     
>     ha_alias_key = ha_alias_key.replace(self.HA_NAMESERVICE_PARAM, 
> ha_nameservice)
>     ha_nameservice_alias = self._get_configuration_value(ha_alias_key)
>     if ha_nameservice_alias is None:
>           logger.warning("[Alert][{0}] HA nameservice value is present but 
> there are no aliases for {1}".format(
>     ```
> 
> Miklos Gergely wrote:
>     This is exactly how it is right now, except that the function checks if 
> ha_nameservice is None before replacing the HA_NAMESERVICE_PARAM with it. So 
> if you are right, then there is no issue here, it works fine, so if HA is not 
> "enabled" then the log would be full of warnings about it's absence.
>     
>     As I see the presence of a high_availability block in the alert means 
> that if HA is "enabled" then this is how it would affect the urls that the 
> alert should use. And the only way to tell if it is actually enabled is to 
> see if these propreties are there.
> 
> Miklos Gergely wrote:
>     Sorry, I just realized that what you've proposed is actually different, 
> and it would solve the warning issue. So if you believe that both 
> ha_nameservice and ha_alias_key must not be None than this is a good fix, 
> although the unit tests suggest that it is valid to have only an ha_alias_key 
> without ha_namespace.

I don't think the unit tests are a good representation of what the correct data 
would be here. They can be changed. From my understanding, the namespace value 
is worthless without aliases as well. Which is why we originally produced a 
warning if we had one but could not resolve the fully-qualified HA keys using 
the alias.


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48589/#review137164
-----------------------------------------------------------


On June 11, 2016, 6:24 p.m., Miklos Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48589/
> -----------------------------------------------------------
> 
> (Updated June 11, 2016, 6:24 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Oliver Szabo, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17180
>     https://issues.apache.org/jira/browse/AMBARI-17180
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> base_alert.py puts a warning into the log if there are properties referenced 
> in the HA nameservice or the alias which are not present in the 
> configuration. The absence of these properties is an indicator that the HA is 
> not enabled, it is not a cause for warning.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/alerts/base_alert.py 6c8ca5a 
> 
> Diff: https://reviews.apache.org/r/48589/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Miklos Gergely
> 
>

Reply via email to