From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: location@lists.ipfire.org
Subject: Re: [PATCH v3 1/2] location-importer: Introduce auxiliary function to
 sanitise ASNs
Date: Mon, 25 Oct 2021 19:48:53 +0100
Message-ID: <879D1B75-57E9-4165-ABB9-C66F69F01963@ipfire.org>
In-Reply-To: <12557f79-b0e0-04f1-99ed-3571eb7c68af@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============8866553087068246475=="
List-Id: <location.lists.ipfire.org>

--===============8866553087068246475==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

> On 23 Oct 2021, at 08:15, Peter M=C3=BCller <peter.mueller(a)ipfire.org> wr=
ote:
>=20
> The second version of this patch does this in a more Pythonic way.

Hmm, almost.

> Signed-off-by: Peter M=C3=BCller <peter.mueller(a)ipfire.org>
> ---
> src/python/location-importer.in | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>=20
> 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
>=20
> +	def _check_parsed_asn(self, asn):
> +		"""
> +			Assistive function to filter Autonomous System Numbers not being suitab=
le
> +			for adding to our database. Returns False in such cases, and True other=
wise.
> +		"""
> +
> +		validranges =3D ((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 =3D (
	(1, 23455),
	(23457, 64495),
	...
)

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

> +
> +		for singlerange in validranges:

You can write this as =E2=80=9Cfor first, last in validranges:=E2=80=9D. That=
 way, you have a new variable that expresses what it is all about.

> +			if singlerange[0] <=3D asn and singlerange[1] >=3D 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 =3D None):
> 		# Get first line to find out what type of block this is
> 		line =3D block[0]
> @@ -829,8 +844,8 @@ class CLI(object):
> 					log.debug("Skipping ARIN AS names line not containing an integer for A=
SN")
> 					continue
>=20
> -				if not ((1 <=3D asn and asn <=3D 23455) or (23457 <=3D asn and asn <=
=3D 64495) or (131072 <=3D asn and asn <=3D 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
>=20
> 				# Skip any AS name that appears to be a placeholder for a different RIR=
 or entity...
> --=20
> 2.26.2


--===============8866553087068246475==--