[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5564: Python 2.7 support for Python postinstall script

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #5564:
URL: https://github.com/apache/trafficcontrol/pull/5564#discussion_r582188576



##########
File path: traffic_ops/install/bin/postinstall.py
##########
@@ -560,48 +703,52 @@ def unmarshal_config(dct: dict) -> Dict[str, List[Question]]:
 
 	return ret
 
-def setup_maxmind(maxmind_answer: str, root: str):
+def setup_maxmind(maxmind_answer, root): # type: (str, str) -> None
 	"""
 	If 'maxmind_answer' is a truthy response ('y' or 'yes' (case-insensitive), sets up a Maxmind
 	database using `wget`.
 	"""
-	if maxmind_answer.casefold() not in {'y', 'yes'}:
+	if maxmind_answer.lower() not in {'y', 'yes'}:
 		logging.info("Not downloading Maxmind data")
 		return
 
 	os.chdir(os.path.join(root, 'opt/traffic_ops/app/public/routing'))
 
+	def failed_download(ip_version):  # type: (int) -> None
+		logging.error("Failed to download MaxMind data")
+		logging.debug("(ipv%d) Exception: %s", ip_version, e)

Review comment:
       `e` is not defined in the scope of this function

##########
File path: traffic_ops/install/bin/postinstall.py
##########
@@ -1167,34 +1338,41 @@ def main(
 			logging.critical("psql is not installed, please install it to continue with database setup")
 			return 1
 
-		try:
-			setup_database_data(conn_str, admin_conf, paramconf, root_dir)
-		except (OSError, subprocess.SubprocessError)as e:
+		def db_connect_failed():
 			logging.error("Failed to set up database: %s", e)
 			logging.error(
-				"Can't connect to the database.  " \
-				"Use the script `/opt/traffic_ops/install/bin/todb_bootstrap.sh` " \
+				"Can't connect to the database.  "
+				"Use the script `/opt/traffic_ops/install/bin/todb_bootstrap.sh` "
 				"on the db server to create it and run `postinstall` again."
 			)
+
+		try:
+			setup_database_data(conn_str, admin_conf, paramconf, root_dir)
+		except (OSError, subprocess.CalledProcessError)as e:
+			db_connect_failed()
+			return -1
+		except subprocess.SubprocessError as e:

Review comment:
       Why split these `excepts` if they have the same handler?




----------------------------------------------------------------
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:
users@infra.apache.org