On Thu, Nov 14, 2013 at 01:43:04PM +0100, Hrvoje Ribicic wrote:
> Creating separate data types could be done, but it really does sound like
> over-engineering. If there were already data types like this, that would be
> a good idea, but otherwise not.
> How about we just remove the FIXME if we do not plan on doing this?

Done.

> 
> On Wed, Nov 13, 2013 at 2:29 PM, Jose A. Lopes <[email protected]> wrote:
> 
> > On Wed, Nov 13, 2013 at 01:44:58PM +0100, Hrvoje Ribicic wrote:
> > > Isn't the FIXME invalid after your change and should be rewritten?
> >
> > The fixme is just saying that the states can be separated into good
> > and bad states.  User down can be seen as either a good or bad state,
> > depending how you argue :) But still, the fixme is valid because we
> > can still group the existing states, instead of having them in one
> > datatype.  However, to me that sounds a bit like over-engineering :)
> >
> > What do you think ?
> >
> > >
> > > LGTM otherwise.
> > >
> > > On Fri, Nov 8, 2013 at 5:24 PM, Jose A. Lopes <[email protected]>
> > wrote:
> > >
> > > > Add instance state 'USER_down' which is a state used in reporting only
> > > > and it represents the situation in which the user has shutdown the
> > > > instance but Ganeti's configuration still has this instance marked as
> > > > 'ADMIN_up'.
> > > >
> > > > Signed-off-by: Jose A. Lopes <[email protected]>
> > > > ---
> > > >  lib/constants.py          | 1 +
> > > >  src/Ganeti/HsConstants.hs | 3 +++
> > > >  src/Ganeti/Types.hs       | 7 +++++--
> > > >  3 files changed, 9 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/constants.py b/lib/constants.py
> > > > index 4ea849a..61eed72 100644
> > > > --- a/lib/constants.py
> > > > +++ b/lib/constants.py
> > > > @@ -989,6 +989,7 @@ INSTST_NODEDOWN = _constants.INSTST_NODEDOWN
> > > >  INSTST_WRONGNODE = _constants.INSTST_WRONGNODE
> > > >  INSTST_ERRORUP = _constants.INSTST_ERRORUP
> > > >  INSTST_ERRORDOWN = _constants.INSTST_ERRORDOWN
> > > > +INSTST_USERDOWN = _constants.INSTST_USERDOWN
> > > >  INSTST_ALL = _constants.INSTST_ALL
> > > >
> > > >  ADMINST_UP = _constants.ADMINST_UP
> > > > diff --git a/src/Ganeti/HsConstants.hs b/src/Ganeti/HsConstants.hs
> > > > index e0185cc..c714f50 100644
> > > > --- a/src/Ganeti/HsConstants.hs
> > > > +++ b/src/Ganeti/HsConstants.hs
> > > > @@ -3033,6 +3033,9 @@ inststNodeoffline = Types.instanceStatusToRaw
> > > > NodeOffline
> > > >  inststRunning :: String
> > > >  inststRunning = Types.instanceStatusToRaw Running
> > > >
> > > > +inststUserdown :: String
> > > > +inststUserdown = Types.instanceStatusToRaw UserDown
> > > > +
> > > >  inststWrongnode :: String
> > > >  inststWrongnode = Types.instanceStatusToRaw WrongNode
> > > >
> > > > diff --git a/src/Ganeti/Types.hs b/src/Ganeti/Types.hs
> > > > index 63124fd..7cef96c 100644
> > > > --- a/src/Ganeti/Types.hs
> > > > +++ b/src/Ganeti/Types.hs
> > > > @@ -329,8 +329,10 @@ $(THH.declareLADT ''String "AllocPolicy"
> > > >         ])
> > > >  $(THH.makeJSONInstance ''AllocPolicy)
> > > >
> > > > --- | The Instance real state type. FIXME: this could be improved to
> > > > --- just wrap a /NormalState AdminStatus | ErrorState ErrorCondition/.
> > > > +-- | The Instance real state type.
> > > > +--
> > > > +-- FIXME: this could be improved to just wrap a /NormalState
> > > > +-- AdminStatus | ErrorState ErrorCondition/.
> > > >  $(THH.declareLADT ''String "InstanceStatus"
> > > >         [ ("StatusDown",    "ADMIN_down")
> > > >         , ("StatusOffline", "ADMIN_offline")
> > > > @@ -339,6 +341,7 @@ $(THH.declareLADT ''String "InstanceStatus"
> > > >         , ("NodeDown",      "ERROR_nodedown")
> > > >         , ("NodeOffline",   "ERROR_nodeoffline")
> > > >         , ("Running",       "running")
> > > > +       , ("UserDown",      "USER_down")
> > > >         , ("WrongNode",     "ERROR_wrongnode")
> > > >         ])
> > > >  $(THH.makeJSONInstance ''InstanceStatus)
> > > > --
> > > > 1.8.4.1
> > > >
> > > >
> > >
> > >
> > > 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
> >
> 
> 
> 
> 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

Reply via email to