Re: [PATCH v3 1/2] location-importer: Introduce auxiliary function to sanitise ASNs

Hello,

On 23 Oct 2021, at 08:15, Peter Müller peter.mueller@ipfire.org wrote:

The second version of this patch does this in a more Pythonic way.

Hmm, almost.

Signed-off-by: Peter Müller peter.mueller@ipfire.org

src/python/location-importer.in | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/python/location-importer.in b/src/python/location-importer.in index da058d3..3ad335f 100644 --- a/src/python/location-importer.in +++ b/src/python/location-importer.in @@ -574,6 +574,21 @@ class CLI(object): # be suitable for libloc consumption... return True

• def _check_parsed_asn(self, asn):

• """
  

• 	Assistive function to filter Autonomous System Numbers not being suitable
  

• 	for adding to our database. Returns False in such cases, and True otherwise.
  

• """
  

• validranges = ((1,23455), (23457,64495), (131072, 4199999999))
  

This should be defined at the top of the file. Otherwise, Python will create and destroy a tuple with these values every time the function is being called. That is a lot of overhead.

I also like writing lists like this as follows:

VALID_ASN_RANGES = ( (1, 23455), (23457, 64495), ... )

That way, changes are easier to read in a diff.

• for singlerange in validranges:
  

You can write this as “for first, last in validranges:”. That way, you have a new variable that expresses what it is all about.

• 	if singlerange[0] <= asn and singlerange[1] >= asn:
  

• 		return True
  

• log.info("Supplied ASN %s out of publicly routable ASN ranges" % asn)
  

• return False
  

• def _parse_block(self, block, source_key, validcountries = None): # Get first line to find out what type of block this is line = block[0]

@@ -829,8 +844,8 @@ class CLI(object): log.debug("Skipping ARIN AS names line not containing an integer for ASN") continue

• 		if not ((1 <= asn and asn <= 23455) or (23457 <= asn and asn <= 64495) or (131072 <= asn and asn <= 4199999999)):
  

• 			log.debug("Skipping ARIN AS names line not containing a valid ASN: %s" % asn)
  

• 		# Filter invalid ASNs...
  

• 		if not self._check_parsed_asn(asn):
  		continue
  
  	# Skip any AS name that appears to be a placeholder for a different RIR or entity...
  

-- 2.26.2