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. > 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
