On Thu, Nov 7, 2013 at 5:30 PM, Klaus Aehlig <[email protected]> wrote:
> On Thu, Nov 07, 2013 at 05:18:34PM +0100, Michele Tartara wrote:
>> On Thu, Nov 7, 2013 at 1:20 PM, Klaus Aehlig <[email protected]> wrote:
>> > When queueing many jobs, the dependencies between them need to
>> > be resolved with the knowledge of their respective job id.
>> > Lift the computation of the absolute dependency to the level
>> > of MetaOpCodes.
>> >
>> > Signed-off-by: Klaus Aehlig <[email protected]>
>> > ---
>> >  src/Ganeti/OpCodes.hs | 14 ++++++++++++++
>> >  1 file changed, 14 insertions(+)
>> >
>> > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
>> > index 82f457f..c1b3048 100644
>> > --- a/src/Ganeti/OpCodes.hs
>> > +++ b/src/Ganeti/OpCodes.hs
>> > @@ -40,6 +40,7 @@ module Ganeti.OpCodes
>> >    , CommonOpParams(..)
>> >    , defOpParams
>> >    , MetaOpCode(..)
>> > +  , resolveDependencies
>> >    , wrapOpCode
>> >    , setOpComment
>> >    , setOpPriority
>> > @@ -1032,11 +1033,24 @@ defOpParams =
>> >                   , opReason     = []
>> >                   }
>> >
>> > +-- | Resolve relative dependencies to absolute ones, given the job ID.
>> > +resolveDepends :: (Monad m) => CommonOpParams -> JobId -> m CommonOpParams
>>
>> Looking at the name it's not clear that this function is just a helper
>> for the resolveDependencies one, nor whether one should use
>> resolveDepends or resolveDependencies, until one goes and looks at the
>> names of the exported functions.
>>
>> What about naming it resolveDepHelper, or resolveDepCommon, to make it
>> more clear what it actually does?
>
> Good idea.
>
>     Interdiff 05/22 Add function to resolve dependencies in meta op code
>
> diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
> index c1b3048..aea3424 100644
> --- a/src/Ganeti/OpCodes.hs
> +++ b/src/Ganeti/OpCodes.hs
> @@ -1034,11 +1034,11 @@ defOpParams =
>                   }
>
>  -- | Resolve relative dependencies to absolute ones, given the job ID.
> -resolveDepends :: (Monad m) => CommonOpParams -> JobId -> m CommonOpParams
> -resolveDepends p@(CommonOpParams { opDepends = Just deps}) jid = do
> +resolveDependsCommon :: (Monad m) => CommonOpParams -> JobId -> m 
> CommonOpParams
> +resolveDependsCommon p@(CommonOpParams { opDepends = Just deps}) jid = do
>    deps' <- mapM (`absoluteJobDependency` jid) deps
>    return p { opDepends = Just deps' }
> -resolveDepends p _ = return p
> +resolveDependsCommon p _ = return p
>
>  -- | The top-level opcode type.
>  data MetaOpCode = MetaOpCode { metaParams :: CommonOpParams
> @@ -1048,7 +1048,7 @@ data MetaOpCode = MetaOpCode { metaParams :: 
> CommonOpParams
>  -- | Resolve relative dependencies to absolute ones, given the job Id.
>  resolveDependencies :: (Monad m) => MetaOpCode -> JobId -> m MetaOpCode
>  resolveDependencies mopc jid = do
> -  mpar <- resolveDepends (metaParams mopc) jid
> +  mpar <- resolveDependsCommon (metaParams mopc) jid
>    return (mopc { metaParams = mpar })
>
>  -- | JSON serialisation for 'MetaOpCode'.
>
>

LGTM, thanks.

Michele

-- 
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

Reply via email to