[v3,1/2] location-importer: Introduce auxiliary function to sanitise ASNs
Commit Message
The second version of this patch does this in a more Pythonic way.
Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
---
src/python/location-importer.in | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
Comments
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
@@ -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))
+
+ for singlerange in validranges:
+ 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...