ocket8888 commented on a change in pull request #3691: Some best-practices 
updates to the kickstart script
URL: https://github.com/apache/trafficcontrol/pull/3691#discussion_r320911626
 
 

 ##########
 File path: misc/kickstart_create_network_line.py
 ##########
 @@ -14,60 +14,56 @@
 #
 
 
-''' 
+'''
     This reads a configuration file and checks for functioning
-    network links in /sys/class/net/*, then emits a ks.cfg network line. 
+    network links in /sys/class/net/*, then emits a ks.cfg network line.
     '''
 
 import os
 import re
 
-# Global bad: 
-global DEBUG
-DEBUG = True
-
 global TO_LOG
 # This "logs" to stdout which is captured during kickstart
 TO_LOG = True
 
-# This is the standard interface we install to. It is set to a speed value of 
-# 5 (vice 100,1000 or 10000) later on and any other interface will override it 
-# if you've got something faster installed. 
+# This is the standard interface we install to. It is set to a speed value of
+# 5 (vice 100,1000 or 10000) later on and any other interface will override it
+# if you've got something faster installed.
 standard_interface=['p4p1']
 
 ## These are configuration settings:
 # Name of Configuration file:
 cfg_file = "network.cfg"
 
-# Where linux is putting the interface stuff: 
+# Where linux is putting the interface stuff:
 iface_dir = '/sys/class/net/'
 
 ignore_interfaces = ['lo','bond']
 
 # Where we kickstart mounts the ISO, and our config directory:
 base_cfg_dir = '/mnt/stage2/ks_scripts/'
 
-# Remember the ? makes the match non-greedy. This is important. 
+# Remember the ? makes the match non-greedy. This is important.
 cfg_line = re.compile("\s*(?P<key>.*?)=(?P<value>.*)\s*$")
 
-# Pick the interface speed for bonding, or "Auto". 
+# Pick the interface speed for bonding, or "Auto".
 # Auto assumes you want the fastest connections with more than 1 interface,
 # Or if there's not 2 interfaces at the same speed you want the fastest.
-# auto is expected to be a string, otherwise use integers: 
-# Speed is in megs. 1000 is 1 gig, 10000 is 10G. 
+# auto is expected to be a string, otherwise use integers:
+# Speed is in megs. 1000 is 1 gig, 10000 is 10G.
 
 iface_speed = 'auto'
 
-restring="%(iface_dir)s(?P<iface>.*)/speed" %vars()
+restring="".join([iface_dir, "(?P<iface>.*)/speed"])
 
 Review comment:
   I remember reading somewhere that the recommended method of string 
concatenation was to use `join`. I just looked it up, though, and it appears 
that what I was thinking of was this line from PEP8:
   
   > _" In performance sensitive parts of the library, the ''.join() form 
should be used instead. This will ensure that concatenation occurs in linear 
time across various implementations."_
   
   So it's probably fine to use `+`/`+=` here since that's not exactly 
_"performance sensitive"_.
   
   As to "fstring"s, I'd rather not break 2.7 compatibility immediately - 
though technically it should be fine because 3.x includes a deprecation notice 
for Python 2. I don't _think_ 2.7 has those.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to