fix issue #255 (_parse_bgp_as regex to match as and asdot)

[bh-rr]

This will not only match AS number (i.e 65001) but also AS number in asdot notation (ie. 65001.10).

[emieli]

Line 82 converts to integer though so even if the regex allows the dot, converting it to int will cause the script to throw an exception and stop executing:

$ python3
>>> match = "65001.10"
>>> print(int(match))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid literal for int() with base 10: '65001.10'

[bh-rr]

Yep, been discussing this on reddit and well and just noticed. Hoped that to be a quick fix, but this seems to be ahead me. So I guess this can be closed and somebody with more experience should try getting this to work, or confirm that this (returning also asdot) will not be implemented so I have to get asdot notation in a different way somewhere in my script.

[colinmacgiolla]

How about if you handle it like this:

match = re.search(r'^router bgp (\d+)\.?(\d+)?', config)
if match.lastindex == 2:
   return dict(bgp_as='.'.join(match.group(1,2)) )
else:
   return dict(bgp_as=int(match.group(1)))

if it is dotASN you return it as a string, and otherwise you'd return the int as currently exists

[bh-rr]

Thanks for the replies, I have updated and tested again and it seems to work now. Do I have to re-open the pull request or is this automatically added?

[dlyssenko]

1. technically, function _parse_bgp_as() could be rewritten as:

def _parse_bgp_as( self, config ):
        as_num = re.search( r'[\d\.]+', config ).group( 0 )
        return { 'bgp_as': int(as_num) if as_num.isnumeric() else as_num }

The reason why RE r'[\d\.]+' suffices is because config is guaranteed to come starting with 'router bgp ...' in prior processing (i.e.: config = self.get_block('^router bgp .*')), thus searching for a first occurrence of the set of digit and dot will guarantee to match on whatever AS notation is.
However, to provide a better code readability, it's probably worth to write it with a lookbehind assertion:

def _parse_bgp_as( self, config ):
        as_num = re.search( r'(?<=^router bgp ).*', config ).group( 0 )
        return { 'bgp_as': int(as_num) if as_num.isnumeric() else as_num }

2. the function _parse_remote_as() should adopt similar rewrite:

def _parse_remote_as( self, config, name ):
        remote_as_re = rf'(?<=neighbor {name} remote-as ).*'
        match = re.search( remote_as_re, config )
        return { 'remote_as': match.group(0) if match else None }

Please amend your PR respectively and I'll approve the changes

[bh-rr]

1. technically, function _parse_bgp_as() could be rewritten as:

def _parse_bgp_as( self, config ):
        as_num = re.search( r'[\d\.]+', config ).group( 0 )
        return { 'bgp_as': int(as_num) if as_num.isnumeric() else as_num }

The reason why RE r'[\d\.]+' suffices is because config is guaranteed to come starting with 'router bgp ...' in prior processing (i.e.: config = self.get_block('^router bgp .*')), thus searching for a first occurrence of the set of digit and dot will guarantee to match on whatever AS notation is. However, to provide a better code readability, it's probably worth to write it with a lookbehind assertion:

def _parse_bgp_as( self, config ):
        as_num = re.search( r'(?<=^router bgp ).*', config ).group( 0 )
        return { 'bgp_as': int(as_num) if as_num.isnumeric() else as_num }

2. the function _parse_remote_as() should adopt similar rewrite:

def _parse_remote_as( self, config, name ):
        remote_as_re = rf'(?<=neighbor {name} remote-as ).*'
        match = re.search( remote_as_re, config )
        return { 'remote_as': match.group(0) if match else None }

Please amend your PR respectively and I'll approve the changes

@dlyssenko code has been changed accordingly.

[dlyssenko]

@bh-rr , could you please fix indentation in _parse_bgp_as()? CI is failing because it's 7 spaces instead of 8.

[bh-rr]

@dlyssenko done (i hope)