>
>
> >
> > +
>
> Extra empty line.
>
Fixed.
>
> > +-- | Collector type
> > +data CollectorType a b =
> > + CollectorSimple (Bool -> ConfigData -> [a] -> IO [(a, b)]) |
> > + CollectorFieldAware (Bool -> ConfigData -> [String] -> [a] -> IO [(a,
> b)])
>
> Just to share my personal preference, I like to tie the constructor
> names to the type name. I especially like the way Template Haskell
> does it, for example,
>
> data Exp
> = ConE
> | LitE
> | VarE
> | ...
>
> As these types are not used often, it might be wiser for the type to be
the first thing the reader sees.
For more-often used types, I will follow this suggestion.
> Anyway, leave the choice of names up to you. In any case,
> indentation:
>
> data ...
> = ...
> | ...
>
>
Fixed.
> > +
> > -- * Helper functions
> >
> > -- | Builds an unknown field definition.
> > @@ -155,7 +161,7 @@ getRequestedJobIDs qfilter =
> > -- | Generic query implementation for resources that are backed by
> > -- some configuration objects.
> > genericQuery :: FieldMap a b -- ^ Field map
> > - -> (Bool -> ConfigData -> [a] -> IO [(a, b)]) -- ^
> Collector
> > + -> CollectorType a b -- ^ Collector
>
> Saying that the 'FieldMap' is the 'Field map' and the 'CollectorType'
> is the 'Collector' isn't very helpful.
>
Updated with more info.
>
> > -> (a -> String) -- ^ Object to name function
> > -> (ConfigData -> Container a) -- ^ Get all objects from
> config
> > -> (ConfigData -> String -> ErrorResult a) -- ^ Lookup
> object
> > @@ -181,7 +187,9 @@ genericQuery fieldsMap collector nameFn configFn
> getFn cfg
> > fobjects <- resultT $ filterM (\n -> evaluateFilter cfg Nothing n
> cfilter)
> > objects
> > -- here run the runtime data gathering...
>
> This comment is weird. Why not just 'Run the runtime data gathering' ?
>
> Wasn't my comment, old code. Fixed it though, and some other surrounding
comments.
Thanks for the review, interdiff:
diff --git a/src/Ganeti/Query/Query.hs b/src/Ganeti/Query/Query.hs
index de0a711..32ce98e 100644
--- a/src/Ganeti/Query/Query.hs
+++ b/src/Ganeti/Query/Query.hs
@@ -83,11 +83,10 @@ import Ganeti.Path
import Ganeti.Types
import Ganeti.Utils
-
-- | Collector type
-data CollectorType a b =
- CollectorSimple (Bool -> ConfigData -> [a] -> IO [(a, b)]) |
- CollectorFieldAware (Bool -> ConfigData -> [String] -> [a] -> IO [(a,
b)])
+data CollectorType a b
+ = CollectorSimple (Bool -> ConfigData -> [a] -> IO [(a, b)])
+ | CollectorFieldAware (Bool -> ConfigData -> [String] -> [a] -> IO [(a,
b)])
-- * Helper functions
@@ -176,8 +175,8 @@ getRequestedJobIDs qfilter =
-- The gathered data, or the failure to get it, is expressed through a
runtime
-- object. The type of a runtime object is determined by every query type
for
-- itself, and used exclusively by that query.
-genericQuery :: FieldMap a b -- ^ Field map
- -> CollectorType a b -- ^ Collector
+genericQuery :: FieldMap a b -- ^ Maps field names to field
definitions
+ -> CollectorType a b -- ^ Collector of live data
-> (a -> String) -- ^ Object to name function
-> (ConfigData -> Container a) -- ^ Get all objects from
config
-> (ConfigData -> String -> ErrorResult a) -- ^ Lookup object
@@ -198,15 +197,15 @@ genericQuery fieldsMap collector nameFn configFn
getFn cfg
[] -> Ok . niceSortKey nameFn .
Map.elems . fromContainer $ configFn cfg
_ -> mapM (getFn cfg) wanted
- -- runs first pass of the filter, without a runtime context; this
- -- will limit the objects that we'll contact for exports
+ -- Run the first pass of the filter, without a runtime context; this will
+ -- limit the objects that we'll contact for exports
fobjects <- resultT $ filterM (\n -> evaluateFilter cfg Nothing n
cfilter)
objects
- -- here run the runtime data gathering...
+ -- Gather the runtime data
runtimes <- case collector of
CollectorSimple collFn -> lift $ collFn live' cfg fobjects
CollectorFieldAware collFn -> lift $ collFn live' cfg fields fobjects
- -- ... then filter again the results, based on gathered runtime data
+ -- ... then filter the results again, based on the gathered data
let fdata = map (\(obj, runtime) ->
map (execGetter cfg runtime obj) fgetters)
runtimes
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
