On Mon, Jan 28, 2013 at 12:07 +0100, Iustin Pop <ius...@google.com> wrote:
> > +-- | Query jobs of a pending repair, returning the new instance data.
> > +processPending :: L.Client -> InstanceData -> IO InstanceData
> > +processPending client instData =
> > + case arState instData of
> > + (ArPendingRepair arData) -> do
> > + sts <- L.queryJobsStatus client $ arJobs arData
> > + time <- getClockTime
> > + case sts of
> > + Bad e -> exitErr $ "could not check job status: " ++ formatError e
> > + Ok sts' -> if any (<= JOB_STATUS_RUNNING) sts' then
> > + return instData -- (no change)
> > + else do
> > + let iname = Instance.name $ arInstance instData
> > + srcSt = arStateName $ arState instData
> > + destSt = arStateName arState'
> > + putStrLn ("Moving " ++ iname ++ " from " ++ show
> > srcSt ++
> > + " to " ++ show destSt)
> > + commitChange client instData'
> Since the 'where' block associated with the if is indented, could you
> move/indent the if block to the next line as well?
Ok. So I assume this is how it should look like?:
case sts of
Bad e -> exitErr $ "could not check job status: " ++ formatError e
Ok sts' ->
if any (<= JOB_STATUS_RUNNING) sts' then
return instData -- (no change)
else do
let iname = Instance.name $ arInstance instData
srcSt = arStateName $ arState instData
destSt = arStateName arState'
putStrLn ("Moving " ++ iname ++ " from " ++ show srcSt ++ " to " ++
show destSt)
commitChange client instData'
where
instData' =
instData { arState = arState'
, tagsToRemove = delCurTag instData
}
arState' =
if all (== JOB_STATUS_SUCCESS) sts' then
ArHealthy $ Just (updateTag $ arData { arResult = Just ArSuccess
, arTime = time })
else
ArFailedRepair (updateTag $ arData { arResult = Just ArFailure
, arTime = time })
> > +-- | Apply and remove tags from an instance as indicated by 'InstanceData'.
> > +--
> > +-- If the /arState/ of the /InstanceData/ record has an associated
> > +-- 'AutoRepairData', add its tag to the instance object. Additionally, if
> > +-- /tagsToRemove/ is not empty, remove those tags from the instance
> > object. The
> > +-- returned /InstanceData/ object always has an empty /tagsToRemove/.
> > +commitChange :: L.Client -> InstanceData -> IO InstanceData
> > +commitChange client instData = do
> > + let iname = Instance.name $ arInstance instData
> > + arData = getArData $ arState instData
> > + rmTags = tagsToRemove instData
> > + execJobsWaitOk' opcodes = do
> > + res <- execJobsWaitOk [map wrapOpCode opcodes] client
> > + case res of
> > + Ok _ -> return ()
> > + Bad e -> exitErr e
> > +
> > + when (isJust arData) $ do
> > + let tag = arTag $ fromJust arData
> Mmm, isJust+fromJust… Sadly the proper case arData of is a bit more
> unreadable.
Ack. I do find easier to read this way.
> > + putStrLn (">>> Adding the following tag to " ++ iname ++ ":\n" ++ show
> > tag)
> > + execJobsWaitOk' [OpTagsSet (TagInstance iname) [tag]]
> > +
> > + unless (null rmTags) $ do
> > + putStr (">>> Removing the following tags from " ++ iname ++ ":\n" ++
> > + unlines (map show rmTags))
> > + execJobsWaitOk' [OpTagsDel (TagInstance iname) rmTags]
> You're executing two jobs here. Would it make more sense to execute one
> job, with two opcodes?
Hmm... I would prefer not to. I didn't consider it in the first place
because I didn't realize the semantics would be exactly the same (i.e.,
second opcode doesn't execute if the first one fails), but I would lose
some precision in regards to the messages that are printed (the
"Removing" message could be printed even when it doesn't get attempted,
or it would be unclear what was executed and what not). One of my goals
was to clearly indicate what the tool is doing with the tags...
I can still change it if you think it's better, but in that case I could
use a bit of help in structuring the code in a way that doesn't suck. I
tried to write a version with a single job, and I didn't manage to make
it non-ugly.
Cheers,
--
Dato Simó | d...@google.com
Corp Fleet Management / Ganeti SRE (Dublin)
--
You received this message because you are subscribed to the Google Groups
"ganeti-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to ganeti-devel+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
