> +extractOpCode :: JobWithStat -> Maybe OpCode
> +extractOpCode job =
> + let qop = listToMaybe . qjOps . jJob $ job
> + metaOps = maybe [] (JQ.toMetaOpCode . qoInput) qop
> + in (fmap metaOpCode) . listToMaybe $ metaOps
As it is only used in a heuristics, it probably is fine; however it
should be at least mentioned in a comment that only the first opcode
of a job is returned (note that a job may be composed of several opcodes).
A better name might be extractFirstOpCode.
> +-- | Check if the opcode type requires the Big Ganeti Lock or not.
> +hasBGL :: OpCode -> Bool
> +hasBGL OpNodeAdd{} = True
> +hasBGL OpNodeRemove{} = True
> +hasBGL OpClusterActivateMasterIp{} = True
> +hasBGL OpClusterDeactivateMasterIp{} = True
> +hasBGL OpClusterDestroy{} = True
> +hasBGL OpClusterPostInit{} = True
> +hasBGL OpClusterRename{} = True
> +hasBGL _ = False
What about OpTestAllocator? Also, it might be worth to add a test to keep
this predicate consistent with REQ_BGL_WHITELIST from
test/py/ganeti.mcpu_unittest.p;
this can happen in a follow up patch.
> @@ -712,6 +740,7 @@ genNodeGroup = do
> serial tags
> return group
>
> +
unrelated whitespace change?
> instance Arbitrary NodeGroup where
> arbitrary = genNodeGroup
>
> --- a/test/hs/Test/Ganeti/TestCommon.hs
> +++ b/test/hs/Test/Ganeti/TestCommon.hs
> @@ -264,16 +264,17 @@ genPrintableAsciiStringNE = do
> -- | Generates a single name component.
> genName :: Gen String
> genName = do
> - n <- choose (1, 16)
> - dn <- vector n
> - return (map dnsGetChar dn)
> + n <- choose (1, 40) :: Gen Int
> + return $ "name" ++ show n
This change is dangerous: genName is also used in tests different from the ones
you added, in particular in ones used to verify encoding and decoding; so using
only those specific names makes those exisiting tests less useful. What is the
purpose of that change?
> -- | Generates an entire FQDN.
> genFQDN :: Gen String
> genFQDN = do
> - ncomps <- choose (1, 4)
> - names <- vectorOf ncomps genName
> - return $ intercalate "." names
> + domain <- ("domain" ++) . show <$> (choose (1, 5) :: Gen Int)
> + cluster <- ("cluster" ++) . show <$> (choose (1, 10) :: Gen Int)
> + tld <- elements [ "com", "net", "org", "gov" ]
> + name <- genName
> + return $ name ++ "." ++ cluster ++ "." ++ domain ++ "." ++ tld
dito
--
Klaus Aehlig
Google Germany GmbH, Erika-Mann-Str. 33, 80636 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Matthew Scott Sucherman, Paul Terence Manicle
