Re: [PATCH master] Making ParticalNic's network field of type String

[Michele Tartara]

On Tue, Feb 5, 2013 at 10:45 AM, Helga Velroyen <hel...@google.com> wrote:

> This is yet another fix for type confusion between python
> and haskell. ;) The network field of PartialNic should be
> a string and not of type Network. This makes it necessary
> to add a helper function to look up a network by name
> and not by UUID.
>
> Signed-off-by: Helga Velroyen <hel...@google.com>
> ---
>  src/Ganeti/Objects.hs       |  2 +-
>  src/Ganeti/Query/Network.hs | 18 ++++++++++++++----
>  2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
> index 26ccea6..388e5cf 100644
> --- a/src/Ganeti/Objects.hs
> +++ b/src/Ganeti/Objects.hs
> @@ -216,7 +216,7 @@ $(buildObject "PartialNic" "nic"
>    [ simpleField "mac" [t| String |]
>    , optionalField $ simpleField "ip" [t| String |]
>    , simpleField "nicparams" [t| PartialNicParams |]
> -  , optionalField $ simpleField "network" [t| Network |]
> +  , optionalField $ simpleField "network" [t| String |]
>    ])
>
>  -- * Disk definitions
> diff --git a/src/Ganeti/Query/Network.hs b/src/Ganeti/Query/Network.hs
> index 6ba0ce4..a951a24 100644
> --- a/src/Ganeti/Query/Network.hs
> +++ b/src/Ganeti/Query/Network.hs
> @@ -125,10 +125,20 @@ getNicLink nic_params = fromMaybe "-" (nicpLinkP
> nic_params)
>  -- | Retrieves the network's instances' names.
>  getInstances :: ConfigData -> String -> [String]
>  getInstances cfg network_uuid =
> -  map instName (filter (instIsConnected network_uuid)
> +  map instName (filter (instIsConnected cfg network_uuid)
>      ((Map.elems . fromContainer . configInstances) cfg))
>
>  -- | Helper function that checks if an instance is linked to the given
> network.
> -instIsConnected :: String -> Instance -> Bool
> -instIsConnected network_uuid inst =
> -  network_uuid `elem` map networkUuid (mapMaybe nicNetwork (instNics
> inst))
> +instIsConnected :: ConfigData -> String -> Instance -> Bool
> +instIsConnected cfg network_uuid inst =
> +  network_uuid `elem` mapMaybe (getNetworkUuid cfg)
> +    (mapMaybe nicNetwork (instNics inst))
>

In general LGTM, but:


> +
> +-- | Helper function to look up a network's UUID by its name
> +getNetworkUuid :: ConfigData -> String -> Maybe String
> +getNetworkUuid cfg name =
> +  let nets = filter (\n -> name == fromNonEmpty (networkName n))
> +               ((Map.elems . fromContainer . configNetworks) cfg)
> +  in case nets of
> +    [] -> Nothing
> +    net:_ -> Just ((fromNonEmpty . networkName) net)
>

Couldn't you implement this by just using
Data.List.find :: (a -> Bool) -> [a] -> Maybe a
instead of filter and case?

Thanks,
Michele