On Wed, Sep 26, 2012 at 05:50:50PM +0200, Agata Murawska wrote:
> On Wed, Sep 26, 2012 at 9:57 AM, Iustin Pop <[email protected]> wrote:
> > On Tue, Sep 25, 2012 at 06:43:54PM +0200, Agata Murawska wrote:
> >> Implementation of node version query.
> >>
> >> Signed-off-by: Agata Murawska <[email protected]>
> >> ---
> >> htools/Ganeti/Rpc.hs | 31 +++++++++++++++++++++++++++++++
> >> 1 files changed, 31 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/htools/Ganeti/Rpc.hs b/htools/Ganeti/Rpc.hs
> >> index d21b834..cb9ee54 100644
> >> --- a/htools/Ganeti/Rpc.hs
> >> +++ b/htools/Ganeti/Rpc.hs
> >> @@ -53,6 +53,9 @@ module Ganeti.Rpc
> >> , RpcCallNodeInfo(..)
> >> , RpcResultNodeInfo(..)
> >>
> >> + , RpcCallVersion(..)
> >> + , RpcResultVersion(..)
> >> +
> >> , rpcTimeoutFromRaw -- FIXME: Not used anywhere
> >> ) where
> >>
> >> @@ -333,3 +336,31 @@ instance RpcResult RpcResultNodeInfo where
> >> Right $ RpcResultNodeInfo boot_id vg_info hv_info
> >>
> >> instance Rpc RpcCallNodeInfo RpcResultNodeInfo
> >> +
> >> +-- | Version
> >> +-- Query node version.
> >> +-- Note: We can't use THH as it does not know what to do with empty dict
> >> +data RpcCallVersion = RpcCallVersion {}
> >> + deriving (Show, Read, Eq)
> >> +
> >> +instance J.JSON RpcCallVersion where
> >> + showJSON _ = J.JSNull
> >> + readJSON _ = fail "Not implemented"
> >
> > For consistency, we should make this accept only JSNull and
> > fail for any other value.
> Ack, not sure if this is what you meant though
Yep, that indeed. I want to be able to use the arbitrary serialisation
helper for this, and the initial version didn't match (readJSON .
showJSON) = id (plus the Ok part)
> >> +$(buildObject "RpcResultVersion" "rpcResultVersion"
> >> + [ simpleField "version" [t| Int |]
> >> + ])
> >> +
> >> +instance RpcCall RpcCallVersion where
> >> + rpcCallName _ = "version"
> >> + rpcCallTimeout _ = rpcTimeoutToRaw Urgent
> >> + rpcCallAcceptOffline _ = True
> >> + rpcCallData call _ = J.encode [call]
> >
> > I wouldn't mind alignment here on "="…
> I will change that but I'd rather do it in the next series, because it
> is not aligned anywhere else
Ah, OK, leave that then.
> >> +instance RpcResult RpcResultVersion where
> >> + rpcResultFill res =
> >> + case J.readJSON res of
> >> + J.Error err -> return . Left $ JsonDecodeError err
> >> + J.Ok ver -> return . Right $ RpcResultVersion ver
> >
> > Hmm, smells of a helper here too…
> Once ERpcResult is a monad it should just be variant of "fromJResult"
> without the string part, for now I just pushed return before case.
Sounds good.
> Interdiff:
> diff --git a/htools/Ganeti/Rpc.hs b/htools/Ganeti/Rpc.hs
> index 678aefe..efa414c 100644
> --- a/htools/Ganeti/Rpc.hs
> +++ b/htools/Ganeti/Rpc.hs
> @@ -345,7 +345,8 @@ data RpcCallVersion = RpcCallVersion {}
>
> instance J.JSON RpcCallVersion where
> showJSON _ = J.JSNull
> - readJSON _ = fail "Not implemented"
> + readJSON J.JSNull = RpcCallVersion
I believe here you need a "return RpcCallVersion"?
> + readJSON _ = fail "Unable to read RpcCallVersion"
>
> $(buildObject "RpcResultVersion" "rpcResultVersion"
> [ simpleField "version" [t| Int |]
> @@ -359,8 +360,8 @@ instance RpcCall RpcCallVersion where
>
> instance RpcResult RpcResultVersion where
> rpcResultFill res =
> - case J.readJSON res of
> - J.Error err -> return . Left $ JsonDecodeError err
> - J.Ok ver -> return . Right $ RpcResultVersion ver
> + return $ case J.readJSON res of
> + J.Error err -> Left $ JsonDecodeError err
> + J.Ok ver -> Right $ RpcResultVersion ver
LGTM, thanks.
iustin
