On Thursday, December 1, 2016 at 11:53:43 AM UTC, Iustin Pop wrote:
>
> On Thu, Dec 01, 2016 at 11:38:35AM +0000, Ganeti Development List wrote: 
> > Following issue #1194, this patch allows Ganeti to correctly 
> > parse drbd versions that also include a dash in their k-fix 
> > version component. 
>
> This means 8.4.8-1 and 8.4.8.1 will be treated the same. Is this the 
> correct behaviour? 
>

Yes, you are correct. I'm not sure to be honest. I've been looking around 
the drbd and 
also the debian packaging site but I'm not 100% sure what logic follows 
their 'k-fix' 
numbering scheme. In drbd, from what I can see, they always use x.y.z-k 
except 
in one particular (old) version where they do x.y.z.k. 

Honestly, though, as somebody correctly pointed out on the ganeti mailing 
list, does it
really matter? Shouldn't we mostly only care about major and minor version 
as that 
should be what might break compatibility? All in all I'd say we can treat 
these as 
interchangeable but if somebody disagrees or knows more about this, I'd be 
happy to 
fix it.
 

>
> > Signed-off-by: Federico Morg Pareschi <mo...@google.com <javascript:>> 
> > --- 
> >  lib/storage/drbd_info.py                | 17 +++++++++++++++-- 
> >  test/py/ganeti.storage.drbd_unittest.py | 10 ++++++++++ 
> >  2 files changed, 25 insertions(+), 2 deletions(-) 
> > 
> > diff --git a/lib/storage/drbd_info.py b/lib/storage/drbd_info.py 
> > index 99605f1..469ed7f 100644 
> > --- a/lib/storage/drbd_info.py 
> > +++ b/lib/storage/drbd_info.py 
> > @@ -164,13 +164,15 @@ class DRBD8Info(object): 
> >   
> >    """ 
> >   
> > -  _VERSION_RE = re.compile(r"^version: (\d+)\.(\d+)\.(\d+)(?:\.(\d+))?" 
> > +  _VERSION_RE = re.compile(r"^version: 
> (\d+)\.(\d+)\.(\d+)(?:[.-](\d+))?" 
> >                             r" \(api:(\d+)/proto:(\d+)(?:-(\d+))?\)") 
> >    _VALID_LINE_RE = re.compile("^ *([0-9]+): cs:([^ ]+).*$") 
> > +  _K_FIX_DASH_SEPARATOR_RE = re.compile(r"^version: 
> (\d+)\.(\d+)\.(\d+)(?:-)") 
> >   
> > +  def _GetKFixSeparator(self, lines): 
> > +    """Check, in case of a K-fix version, if the separator is a dash or 
> dot.""" 
> > + 
> > +    first_line = lines[0].strip() 
> > +    match = self._K_FIX_DASH_SEPARATOR_RE.match(first_line) 
> > +    if match is None: 
> > +      return "." 
> > +    else: 
> > +      return "-" 
>
> This seems to be done in two steps. Would it be simpler to have 
> K_FIX_DASH_SEPARATOR itself extract the separator, instead of it having 
> to match - and if not, return . vs -? 
>
> You could even get rid of _K_FIX_DASH_SEPARATOR_RE, and extract the 
> separator from the _VERSION_RE, after changing the RE to have the 
> separator a capturing group. 
>

I was trying to get as little regex pain as possible but what you're saying 
actually
makes more sense, I'll see what I can do with capture groups and fix the 
code 
accordingly, thanks.

Morg.

Reply via email to