On Tue, Sep 25, 2012 at 06:43:52PM +0200, Agata Murawska wrote:
> The tests we currently have assume, that all the data required for
> running the query is available - once we add live data, this will no
> longer be the case.
>
> This patch adds boolean parameter to query function, which tells it
> whether to ignore live parameters gathering.
I would have split this in two parts: first, the addition of the live
field, and then the integration, as they are two separate things.
> +-- | Collect live data from RPC query if enabled.
> +maybeCollectLiveData:: Bool -> ConfigData -> [Node] -> IO [(Node,
> NodeRuntime)]
> +
> +maybeCollectLiveData False _ nodes =
> + return $ zip nodes (repeat $ Left (RpcResultError "Live data disabled"))
> +
> +maybeCollectLiveData True cfg nodes = do
> + let vgs = [clusterVolumeGroupName $ configCluster cfg]
> + hvs = clusterEnabledHypervisors $ configCluster cfg
> + executeRpcCall nodes (RpcCallNodeInfo vgs hvs)
This is the part which we discussed offline that can be improved.
maybeCollectLiveData should take the list of requested fields, and
decide whether to ask for the hv info and/or vg info based on the
requested fields. Please add a FIXME for this.
> +-- | Check whether list of queried fields contains live fields.
> +needsLiveData :: [FieldGetter a b] -> Bool
> +needsLiveData [] = False
> +needsLiveData (FieldRuntime _:_) = True
> +needsLiveData (_:xs) = needsLiveData xs
Uh… you seem to really like explicit recursion, which is badstyle™:
needsLiveData = any (\getter -> case getter of
FieldRuntime _ -> True
_ -> False)
Rest looks good.
thanks,
iustin
