LGTM. Thanks, Jose
On Tue, Oct 15, 2013 at 02:19:38PM +0200, Hrvoje Ribicic wrote: > On Mon, Oct 14, 2013 at 6:38 PM, Jose A. Lopes <[email protected]> wrote: > > > On Mon, Oct 14, 2013 at 10:25:27AM +0200, Hrvoje Ribicic wrote: > > > This commit adds the instance network fields. Some of these fields > > > are also multiplied per-NIC, requiring the reuse of functions from > > > the previous disk instance fields commit. > > > > 'multiplied' ? > > what does this mean? > > > > > Present for every NIC. Which is easier to understand, so I am using that > instead. > > > > > > +-- | Gets the bridge of a NIC. > > > +getNicBridge :: FilledNicParams -> Maybe String > > > +getNicBridge nicParams = > > > + if nicpMode nicParams == NMBridged > > > + then Just $ nicpLink nicParams > > > + else Nothing > > > > Pattern guards ? > > > > Done. > > > > > +-- | Creates a function which produces a FieldGetter when fed an index. > > Works > > > +-- for fields that should be filled out through the use of a default. > > > > Quote 'FieldGetter'. > > > > > +getFillableFieldWithDefault :: (J.JSON c) > > > + => (Instance -> [a]) -- ^ Extracts a list of incomplete > > objects > > > + -> (ConfigData -> Instance -> b) -- ^ Extracts the default object > > > + -> (b -> a -> b) -- ^ Fills the default object > > > + -> (b -> Maybe c) -- ^ Extracts an obj property > > > + -> Int -- ^ Index in list to use > > > + -> FieldGetter Instance Runtime -- ^ Result > > > +getFillableFieldWithDefault > > > + listGetter defaultGetter fillFn propertyGetter index = > > > + FieldConfig(\cfg inst -> rsMaybeUnavail $ do > > > + incompleteObj <- maybeAt index $ > > listGetter inst > > > + let defaultObj = defaultGetter cfg inst > > > + completeObj = fillFn defaultObj > > incompleteObj > > > + propertyGetter completeObj) > > > + > > > > Missing space between 'FieldConfig' and parenthesis. > > > > > Fixed. > > Thanks for the review, interdiff is: > > diff --git a/src/Ganeti/Query/Instance.hs b/src/Ganeti/Query/Instance.hs > index 4bca54e..81f8ec8 100644 > --- a/src/Ganeti/Query/Instance.hs > +++ b/src/Ganeti/Query/Instance.hs > @@ -189,55 +189,46 @@ instanceFields = > -- Aggregate nic parameter fields > [ (FieldDefinition "nic.count" "NICs" QFTNumber > "Number of network interfaces", > - FieldSimple (rsNormal . length . instNics), QffNormal), > - > - (FieldDefinition "nic.macs" "NIC_MACs" QFTOther > + FieldSimple (rsNormal . length . instNics), QffNormal) > + , (FieldDefinition "nic.macs" "NIC_MACs" QFTOther > (nicAggDescPrefix ++ "MAC address"), > - FieldSimple (rsNormal . map nicMac . instNics), QffNormal), > - > - (FieldDefinition "nic.ips" "NIC_IPs" QFTOther > + FieldSimple (rsNormal . map nicMac . instNics), QffNormal) > + , (FieldDefinition "nic.ips" "NIC_IPs" QFTOther > (nicAggDescPrefix ++ "IP address"), > - FieldSimple (rsNormal . map (JSONFriendlyMaybe . nicIp) . instNics), > - QffNormal), > - > - (FieldDefinition "nic.names" "NIC_Names" QFTOther > + FieldSimple (rsNormal . map (MaybeForJSON . nicIp) . instNics), > + QffNormal) > + , (FieldDefinition "nic.names" "NIC_Names" QFTOther > (nicAggDescPrefix ++ "name"), > - FieldSimple (rsNormal . map (JSONFriendlyMaybe . nicName) . instNics), > - QffNormal), > - > - (FieldDefinition "nic.uuids" "NIC_UUIDs" QFTOther > + FieldSimple (rsNormal . map (MaybeForJSON . nicName) . instNics), > + QffNormal) > + , (FieldDefinition "nic.uuids" "NIC_UUIDs" QFTOther > (nicAggDescPrefix ++ "UUID"), > - FieldSimple (rsNormal . map nicUuid . instNics), QffNormal), > - > - (FieldDefinition "nic.modes" "NIC_modes" QFTOther > + FieldSimple (rsNormal . map nicUuid . instNics), QffNormal) > + , (FieldDefinition "nic.modes" "NIC_modes" QFTOther > (nicAggDescPrefix ++ "mode"), > FieldConfig (\cfg -> rsNormal . map > (nicpMode . fillNicParamsFromConfig cfg . nicNicparams) > . instNics), > - QffNormal), > - > - (FieldDefinition "nic.bridges" "NIC_bridges" QFTOther > + QffNormal) > + , (FieldDefinition "nic.bridges" "NIC_bridges" QFTOther > (nicAggDescPrefix ++ "bridge"), > - FieldConfig (\cfg -> rsNormal . map (JSONFriendlyMaybe . getNicBridge > . > + FieldConfig (\cfg -> rsNormal . map (MaybeForJSON . getNicBridge . > fillNicParamsFromConfig cfg . nicNicparams) . instNics), > - QffNormal), > - > - (FieldDefinition "nic.links" "NIC_links" QFTOther > + QffNormal) > + , (FieldDefinition "nic.links" "NIC_links" QFTOther > (nicAggDescPrefix ++ "link"), > FieldConfig (\cfg -> rsNormal . map > (nicpLink . fillNicParamsFromConfig cfg . nicNicparams) > . instNics), > - QffNormal), > - > - (FieldDefinition "nic.networks" "NIC_networks" QFTOther > + QffNormal) > + , (FieldDefinition "nic.networks" "NIC_networks" QFTOther > "List containing each interface's network", > - FieldSimple (rsNormal . map (JSONFriendlyMaybe . nicNetwork) . > instNics), > - QffNormal), > - > - (FieldDefinition "nic.networks.names" "NIC_networks_names" QFTOther > + FieldSimple (rsNormal . map (MaybeForJSON . nicNetwork) . instNics), > + QffNormal) > + , (FieldDefinition "nic.networks.names" "NIC_networks_names" QFTOther > "List containing the name of each interface's network", > FieldConfig (\cfg -> rsNormal . map > - (\x -> JSONFriendlyMaybe (getNetworkName cfg <$> nicNetwork x)) > + (\x -> MaybeForJSON (getNetworkName cfg <$> nicNetwork x)) > . instNics), > QffNormal) > ] ++ > @@ -246,37 +237,29 @@ instanceFields = > fillNumberFields C.maxNics > [ (fieldDefinitionCompleter "nic.ip/%d" "NicIP/%d" QFTText > ("IP address" ++ nicDescSuffix), > - getFillableOptionalField instNics nicIp, QffNormal), > - > - (fieldDefinitionCompleter "nic.uuid/%d" "NicUUID/%d" QFTText > + getFillableOptionalField instNics nicIp, QffNormal) > + , (fieldDefinitionCompleter "nic.uuid/%d" "NicUUID/%d" QFTText > ("UUID address" ++ nicDescSuffix), > - getFillableField instNics nicUuid, QffNormal), > - > - (fieldDefinitionCompleter "nic.mac/%d" "NicMAC/%d" QFTText > + getFillableField instNics nicUuid, QffNormal) > + , (fieldDefinitionCompleter "nic.mac/%d" "NicMAC/%d" QFTText > ("MAC address" ++ nicDescSuffix), > - getFillableField instNics nicMac, QffNormal), > - > - (fieldDefinitionCompleter "nic.name/%d" "NicName/%d" QFTText > + getFillableField instNics nicMac, QffNormal) > + , (fieldDefinitionCompleter "nic.name/%d" "NicName/%d" QFTText > ("Name address" ++ nicDescSuffix), > - getFillableOptionalField instNics nicName, QffNormal), > - > - (fieldDefinitionCompleter "nic.network/%d" "NicNetwork/%d" QFTText > + getFillableOptionalField instNics nicName, QffNormal) > + , (fieldDefinitionCompleter "nic.network/%d" "NicNetwork/%d" QFTText > ("Network" ++ nicDescSuffix), > - getFillableOptionalField instNics nicNetwork, QffNormal), > - > - (fieldDefinitionCompleter "nic.mode/%d" "NicMode/%d" QFTText > + getFillableOptionalField instNics nicNetwork, QffNormal) > + , (fieldDefinitionCompleter "nic.mode/%d" "NicMode/%d" QFTText > ("Mode" ++ nicDescSuffix), > - getFillableNicField nicpMode, QffNormal), > - > - (fieldDefinitionCompleter "nic.link/%d" "NicLink/%d" QFTText > + getFillableNicField nicpMode, QffNormal) > + , (fieldDefinitionCompleter "nic.link/%d" "NicLink/%d" QFTText > ("Link" ++ nicDescSuffix), > - getFillableNicField nicpLink, QffNormal), > - > - (fieldDefinitionCompleter "nic.network.name/%d" "NicNetworkName/%d" > QFTText > + getFillableNicField nicpLink, QffNormal) > + , (fieldDefinitionCompleter "nic.network.name/%d" "NicNetworkName/%d" > QFTText > ("Network name" ++ nicDescSuffix), > - getFillableNicNetworkNameField, QffNormal), > - > - (fieldDefinitionCompleter "nic.bridge/%d" "NicBridge/%d" QFTText > + getFillableNicNetworkNameField, QffNormal) > + , (fieldDefinitionCompleter "nic.bridge/%d" "NicBridge/%d" QFTText > ("Bridge" ++ nicDescSuffix), > getOptionalFillableNicField getNicBridge, QffNormal) > ] ++ > @@ -314,10 +297,9 @@ getNetworkName cfg = networkName . (Map.!) > (fromContainer $ configNetworks cfg) > > -- | Gets the bridge of a NIC. > getNicBridge :: FilledNicParams -> Maybe String > -getNicBridge nicParams = > - if nicpMode nicParams == NMBridged > - then Just $ nicpLink nicParams > - else Nothing > +getNicBridge nicParams > + | nicpMode nicParams == NMBridged = Just $ nicpLink nicParams > + | otherwise = Nothing > > -- | Fill partial NIC params by using the defaults from the configuration. > fillNicParamsFromConfig :: ConfigData -> PartialNicParams -> > FilledNicParams > @@ -353,7 +335,7 @@ getOptionalFillableNicField = > getFillableFieldWithDefault > (map nicNicparams . instNics) (\x _ -> getDefaultNicParams x) > fillNicParams > > --- | Creates a function which produces a FieldGetter when fed an index. > Works > +-- | Creates a function which produces a 'FieldGetter' when fed an index. > Works > -- for fields that should be filled out through the use of a default. > getFillableFieldWithDefault :: (J.JSON c) > => (Instance -> [a]) -- ^ Extracts a list of incomplete > objects > @@ -364,13 +346,13 @@ getFillableFieldWithDefault :: (J.JSON c) > -> FieldGetter Instance Runtime -- ^ Result > getFillableFieldWithDefault > listGetter defaultGetter fillFn propertyGetter index = > - FieldConfig(\cfg inst -> rsMaybeUnavail $ do > - incompleteObj <- maybeAt index $ listGetter > inst > - let defaultObj = defaultGetter cfg inst > - completeObj = fillFn defaultObj > incompleteObj > - propertyGetter completeObj) > + FieldConfig (\cfg inst -> rsMaybeUnavail $ do > + incompleteObj <- maybeAt index $ listGetter > inst > + let defaultObj = defaultGetter cfg inst > + completeObj = fillFn defaultObj > incompleteObj > + propertyGetter completeObj) > > --- | Creates a function which produces a FieldGetter when fed an index. > Works > +-- | Creates a function which produces a 'FieldGetter' when fed an index. > Works > -- for fields that may not return a value, expressed through the Maybe > monad. > getFillableOptionalField :: (J.JSON b) > => (Instance -> [a]) -- ^ Extracts a list of > objects > @@ -383,7 +365,7 @@ getFillableOptionalField extractor optPropertyGetter > index = > obj <- maybeAt index $ extractor inst > optPropertyGetter obj) > > --- | Creates a function which produces a FieldGetter when fed an index. > +-- | Creates a function which produces a 'FieldGetter' when fed an index. > -- Works only for fields that surely return a value. > getFillableField :: (J.JSON b) > => (Instance -> [a]) -- ^ Extracts a list of objects > > > 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 -- Jose Antonio Lopes 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
