> Optional suggestions present, else LGTM. > > @@ -216,8 +218,9 @@ getOnlineNodes = filter (not . nodeOffline) . F.toList > > . configNodes > > -- | Returns the default cluster link. > > getDefaultNicLink :: ConfigData -> String > > getDefaultNicLink = > > - nicpLink . (M.! C.ppDefault) . fromContainer . > > - clusterNicparams . configCluster > > + let ppDefault = UTF8.fromString C.ppDefault > > + in nicpLink . (M.! ppDefault) . fromContainer > > + . clusterNicparams . configCluster > > > > Since this pattern repeats itself: would it make sense to make the constant > a ByteString and modify our Python-generating code to handle this > adequately?
I considered it, but I think it would change infra structure too much, especially for achange that we wouldn't benefit from at run time. We can think about extending the python code generation, but if we do so, it definitely should be a separate patch. If you think it is really important, I can, of course change it; but my preference is to not change it. > Replace key array creation with map lookup? That would be the following interdiff. I'm a bit worried it might change the test itself too much, but if you think it's fine, I'll include it. diff --git a/test/hs/Test/Ganeti/Query/Network.hs b/test/hs/Test/Ganeti/Query/Network.hs index 6e01825..aee0376 100644 --- a/test/hs/Test/Ganeti/Query/Network.hs +++ b/test/hs/Test/Ganeti/Query/Network.hs @@ -69,9 +69,9 @@ prop_getGroupConnection group = -- yields 'Nothing'. prop_getGroupConnection_notFound :: NodeGroup -> String -> Property prop_getGroupConnection_notFound group uuid = - let net_keys = map UTF8.toString . Map.keys . fromContainer . groupNetworks - $ group - in notElem uuid net_keys ==> isNothing (getGroupConnection uuid group) + let net_map = fromContainer . groupNetworks $ group + in not (UTF8.fromString uuid `Map.member` net_map) + ==> isNothing (getGroupConnection uuid group) -- Klaus Aehlig Google Germany GmbH, Dienerstr. 12, 80331 Muenchen Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschaeftsfuehrer: Matthew Scott Sucherman, Paul Terence Manicle
