Code review comment for lp:~allenap/gwacl/remove-role-endpoints

Raphaël Badin (rvb) wrote :

> > [2]
> >
> My thinking goes something like this:
>
> - Always ignore LoadBalancerProbe for the purposes of comparison.
>
> - If LoadBalancedEndpointSetName is defined, then LocalPort, Port,
>  Protocol and VIP should be used for comparison.
>
> - If LoadBalancedEndpointSetName is *not* defined, then Port, Protocol
>  and VIP should be used for comparison.

Why omitting LocalPort in this case? I must say I'm a bit confused about what LoadBalancedEndpointSetName means… even after reading the doc :).
>
> - This leaves Name. I'm not sure what purpose this serves, other than
>  as a pretty name for the UI. If so, it's irrelevant for the purposes
>  of comparison.
>
> It may be worth having a FilterRoleEndpoints function, which behaves
> like RemoveRoleEndpoints, but accepts a func(*InputEndpoint) bool
> function which can either mutate the endpoint or remove it (by
> returning false). I think this might better support the desired
> behaviour in juju-core, and make us more flexible to change.

Seems a bit overkill *if* we can figure out what the "natural key" of an endpoint is…

« Back to merge proposal