2012/10/10 Iustin Pop <[email protected]>:
> On Fri, Oct 05, 2012 at 08:29:28PM +0200, Iustin Pop wrote:
>> On Fri, Oct 05, 2012 at 09:42:34AM +0200, Agata Murawska wrote:
>> > 2012/10/5 Iustin Pop <[email protected]>:
>> > > The String parameter to 'nodeLiveFieldExtract' is the query2 field
>> > > name, not the RPC-layer field name. Grrr for not having a real data
>> > > type for this.
>> > Huh, this is interesting - I had it with FieldName originally, but
>> > changed since the string values here were the same as names of the
>> > fields used for dictionary creation (see Rpc.hs) and I hoped this can
>> > then be at some point automated.
>>
>> I think you are mistaken. The bug was exactly that you used the same
>> string, when the opcode-layer and RPC-layer strings are different.
>>
>> > > Furthermore, we add some safety check that we don't return JSNull via
>> > > rsNormal…
>> > >
>> > > Signed-off-by: Iustin Pop <[email protected]>
>> > > ---
>> > >  htools/Ganeti/Query/Node.hs |   38 
>> > > ++++++++++++++++++++------------------
>> > >  1 file changed, 20 insertions(+), 18 deletions(-)
>> > >
>> > > diff --git a/htools/Ganeti/Query/Node.hs b/htools/Ganeti/Query/Node.hs
>> > > index ded9979..0630754 100644
>> > > --- a/htools/Ganeti/Query/Node.hs
>> > > +++ b/htools/Ganeti/Query/Node.hs
>> > > @@ -72,29 +72,31 @@ nodeLiveFieldsDefs =
>> > >  -- the RPC result.
>> > >  nodeLiveFieldExtract :: String -> RpcResultNodeInfo -> J.JSValue
>> > I may have gaps in my memory (no code on this machine ;) ), but
>> > shouldn't the type be FieldName not String now?
>>
>> Indeed, but type FieldName = String, so I didn't pay too much attention.
>> Consider it fixed.
>
> Ping? Just to be clear, interdiff:

I don't think I still have LGTM powers, but if I do then LGTM :)

>
> diff --git a/htools/Ganeti/Query/Node.hs b/htools/Ganeti/Query/Node.hs
> index 0630754..62b625b 100644
> --- a/htools/Ganeti/Query/Node.hs
> +++ b/htools/Ganeti/Query/Node.hs
> @@ -70,7 +70,7 @@ nodeLiveFieldsDefs =
>
>  -- | Map each name to a function that extracts that value from
>  -- the RPC result.
> -nodeLiveFieldExtract :: String -> RpcResultNodeInfo -> J.JSValue
> +nodeLiveFieldExtract :: FieldName -> RpcResultNodeInfo -> J.JSValue
>  nodeLiveFieldExtract "bootid" res =
>    J.showJSON $ rpcResNodeInfoBootId res
>  nodeLiveFieldExtract "cnodes" res =
>
> --
> thanks,
> iustin

Reply via email to