On Wed, Oct 26, 2011 at 12:43 PM, Iustin Pop <[email protected]> wrote:
> On Wed, Oct 26, 2011 at 11:58:37AM +0200, Agata Murawska wrote:
>> On Wed, Oct 26, 2011 at 10:17 AM, Iustin Pop <[email protected]> wrote:
>> > On Tue, Oct 25, 2011 at 04:54:06PM +0200, Agata Murawska wrote:
>> >> The status field in response to a generic query is now supported, with
>> >> error status used to provide user with more information.
>> >>
>> >> Note:
>> >> The code in THH.hs is temporary, as generalization of declareIADT and
>> >> declareSADT is in the next patch.
>> >>
>> >> Signed-off-by: Agata Murawska <[email protected]>
>> >> ---
>
>> >> @@ -135,16 +141,17 @@ getInstances ktn arr = extractArray arr >>= mapM 
>> >> (parseInstance ktn)
>> >>
>> >>  -- | Construct an instance from a JSON object.
>> >>  parseInstance :: NameAssoc
>> >> -              -> JSValue
>> >> +              -> [(JSValue, JSValue)]
>> >>                -> Result (String, Instance.Instance)
>> >
>> > Side-note: I wonder how will this fail if we don't get an JSArray…
>> JSArray where? The second argument is no longer JSArray...
>
> But that's exactly what I mean :)
>
> What happens if what Ganeti responds with a JSObject, for example?
> Before, we had the explicit case on JSArray [] and _, but now the
> explicit message in _ is no longer called for JSObject for example.

Is there any way to check it? At first I thought that if I'd use the
old querying interface (i.e. change queryInstancesMsg to the old
version using QueryInstances), this would do it, but this is
apparently not the case ;) since it then fails as early as on getData
function. Also, I do not really see the usecase - change in encoding
of luxi response?
I'd like to know more because the way to fix it is ugly, as we'd need
JSArray with elements being JSArray as well..

>
>> Thanks for the review! The interdiff is:
>>
>> index 8ed4761..95f839b 100644
>> --- a/htools/Ganeti/HTools/Luxi.hs
>> +++ b/htools/Ganeti/HTools/Luxi.hs
>> @@ -73,11 +73,11 @@ extractArray :: (Monad m) => JSValue -> m
>> [[(JSValue, JSValue)]]
>>  extractArray v =
>>    getData v >>= parseQueryResult
>>
>> --- | Testing responce status for more verbose error message.
>> -fromJValWithStatus :: (Text.JSON.JSON a, Monad m) => JSValue -> JSValue -> 
>> m a
>> -fromJValWithStatus st v = do
>> -    st' <- fromJVal st
>> -    L.checkRS st' v >>= fromJVal
>> +-- | Testing result status for more verbose error message.
>> +fromJValWithStatus :: (Text.JSON.JSON a, Monad m) => (JSValue, JSValue) -> 
>> m a
>> +fromJValWithStatus v = do
>> +    st <- fromJVal $ fst v
>> +    L.checkRS st (snd v) >>= fromJVal
>
> pattern match directly in the call:
>
> fromJValWithStatus (st, v) = do
>   st' <- …
>
> rest LGTM, thanks.
>
> iustin
>

Reply via email to