summaryrefslogtreecommitdiffstats
path: root/src/live
diff options
context:
space:
mode:
authorAlejandro Sirgo Rica <asirgo@soleta.eu>2024-03-18 14:17:12 +0100
committerlupoDharkael <izhe@hotmail.es>2024-03-21 10:29:57 +0100
commit2a4ce65a20b41a670b274cb473d46e62b4f3c913 (patch)
tree947c45ef966b226cbb980beabdc964184aa81a02 /src/live
parent0cbf16461e05bc5f15d3436763667b1d34869826 (diff)
src: centralize error logging into send_internal_server_error
Use only the exception messages as the main resource for error messages. The previous error code had string duplication in the form of: logging.error('msg here') raise Exception('msg here') That approach also has the downside of having log duplication as it had the local logging.err() and a global logging.exception() inside send_internal_server_error capturing the exception message. The actual code only requires raising an exception with a proper error message. Improve exception messages to give more error context. Log every AssertionError as a backtrace. Use the 'raise Exception from e' syntax to modify the a previously raised exception 'e' into an exception with aditional context or different type. This also prevents the message that warns about newer exceptions being launch after an initial exception.
Diffstat (limited to 'src/live')
-rw-r--r--src/live/ogOperations.py59
1 files changed, 21 insertions, 38 deletions
diff --git a/src/live/ogOperations.py b/src/live/ogOperations.py
index acb1a0e..d564ea9 100644
--- a/src/live/ogOperations.py
+++ b/src/live/ogOperations.py
@@ -51,9 +51,8 @@ class OgLiveOperations:
try:
proc = subprocess.call(["pkill", "-9", "browser"])
proc = subprocess.Popen(["browser", "-qws", url])
- except:
- logging.exception('Cannot restart browser')
- raise ValueError('Error: cannot restart browser')
+ except Exception as e:
+ raise RuntimeError('Cannot restart browser') from e
def _refresh_payload_disk(self, cxt, part_setup, num_disk):
part_setup['disk'] = str(num_disk)
@@ -120,8 +119,7 @@ class OgLiveOperations:
def _write_md5_file(self, path):
if not os.path.exists(path):
- logging.error('Invalid path in _write_md5_file')
- raise ValueError('Invalid image path when computing md5 checksum')
+ raise ValueError(f'Invalid image path {path} when computing md5 checksum')
filename = path + ".full.sum"
dig = self._compute_md5(path)
try:
@@ -145,15 +143,13 @@ class OgLiveOperations:
try:
r = shutil.copy(src, dst)
tip_write_csum(image_name)
- except:
- logging.exception('Error copying image to cache')
- raise ValueError(f'Error: Cannot copy image {image_name} to cache')
+ except Exception as e:
+ raise RuntimeError(f'Error copying image {image_name} to cache. Reported: {e}') from e
def _restore_image_unicast(self, repo, name, devpath, cache=False):
if ogChangeRepo(repo, smb_user=self._smb_user, smb_pass=self._smb_pass) != 0:
self._restartBrowser(self._url)
- logging.error('ogChangeRepo could not change repository to %s', repo)
- raise ValueError(f'Error: Cannot change repository to {repo}')
+ raise ValueError(f'Cannot change repository to {repo}')
logging.debug(f'restore_image_unicast: name => {name}')
if cache:
image_path = f'/opt/opengnsys/cache/opt/opengnsys/images/{name}.img'
@@ -191,8 +187,7 @@ class OgLiveOperations:
cmd_pc = shlex.split(f'partclone.restore -d0 -C -I -o {devpath}')
if not os.path.exists(image_path):
- logging.error('f{image_path} does not exist, exiting.')
- raise ValueError(f'Error: Image not found at {image_path}')
+ raise RuntimeError(f'Image not found at {image_path} during image restore')
with open('/tmp/command.log', 'wb', 0) as logfile:
proc_lzop = subprocess.Popen(cmd_lzop,
@@ -248,9 +243,8 @@ class OgLiveOperations:
shell=True,
executable=OG_SHELL)
(output, error) = ogRest.proc.communicate()
- except:
- logging.exception('Exception when running "shell run" subprocess')
- raise ValueError('Error: Incorrect command value')
+ except Exception as e:
+ raise RuntimeError(f'Error when running "shell run" subprocess: {e}') from e
if ogRest.proc.returncode != 0:
logging.warn('Non zero exit code when running: %s', ' '.join(cmds))
@@ -298,9 +292,8 @@ class OgLiveOperations:
try:
inventory = get_hardware_inventory()
- except ValueError as e:
- logging.exception('Error occurred while running get_hardware_inventory')
- raise e
+ except Exception as e:
+ raise RuntimeError(f'Error while running hardware inventory. {e}') from e
finally:
self._restartBrowser(self._url)
@@ -327,7 +320,6 @@ class OgLiveOperations:
elif table_type == 'GPT':
cxt.create_disklabel('gpt')
else:
- logging.info(f'Unsupported partition layout: {table_type}, only MSDOS and GPT are supported')
raise ValueError(f'Unsupported partition scheme {table_type}, only MSDOS and GPT are supported')
logging.info(f'Setting up partition layout to {table_type}')
@@ -422,8 +414,7 @@ class OgLiveOperations:
if ogChangeRepo(repo, smb_user=self._smb_user, smb_pass=self._smb_pass) != 0:
self._restartBrowser(self._url)
- logging.error('ogChangeRepo could not change repository to %s', repo)
- raise ValueError(f'Error: Cannot change repository to {repo}')
+ raise RuntimeError(f'Cannot change image repository to {repo}')
if ogRest.terminated:
return
@@ -439,30 +430,25 @@ class OgLiveOperations:
if pa is None:
self._restartBrowser(self._url)
- logging.error(f'Target partition /dev/{diskname} not found')
- raise ValueError(f'Target partition /dev/{diskname} not found')
+ raise RuntimeError(f'Target partition /dev/{diskname} not found')
padev = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_DEVICE)
fstype = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_FSTYPE)
if not fstype:
- logging.error(f'No filesystem detected in {padev}. Aborting image creation.')
- raise ValueError('Target partition has no filesystem present')
+ raise RuntimeError(f'No filesystem detected in {padev}. Aborting image creation')
if change_access(user=self._smb_user, pwd=self._smb_pass) == -1:
- logging.error('remount of /opt/opengnsys/images has failed')
- raise AssertionError('remount of /opt/opengnsys/images has failed')
+ raise RuntimeError('remount of /opt/opengnsys/images has failed')
if os.access(f'/opt/opengnsys/images', os.R_OK | os.W_OK) == False:
- logging.error('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
- raise ValueError('Cannot access image folder')
+ raise RuntimeError('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
if os.access(f'{image_path}', os.R_OK) == True:
logging.info(f'image file {image_path} already exists, updating.')
ogCopyEfiBootLoader(disk, partition)
if ogReduceFs(disk, partition) == -1:
- logging.error(f'Failed to shrink {fstype} filesystem in {padev}')
- raise ValueError('Failed to shrink filesystem')
+ raise ValueError(f'Failed to shrink {fstype} filesystem in {padev}')
cmd1 = shlex.split(f'partclone.{fstype} -I -C --clone -s {padev} -O -')
cmd2 = shlex.split(f'lzop -1 -fo {image_path}')
@@ -482,7 +468,7 @@ class OgLiveOperations:
try:
retdata = p2.communicate()
except OSError as e:
- logging.exception('Unexpected error when running partclone and lzop commands')
+ raise OSError(f'Unexpected error when running partclone and lzop commands: {e}') from e
finally:
logfile.close()
p2.terminate()
@@ -494,14 +480,12 @@ class OgLiveOperations:
ogExtendFs(disk, partition)
if os.access(f'{image_path}', os.R_OK) == False:
- logging.error(f'Cannot access partclone image file {image_path}')
- raise ValueError('Cannot access image file')
+ raise RuntimeError(f'Cannot access partclone image file {image_path}')
image_info = ogGetImageInfo(image_path)
except:
self._restartBrowser(self._url)
- logging.exception('Exception when running "image create" subprocess')
- raise ValueError(f'Error: Cannot create image for {fstype} filesystem in device {padev}')
+ raise RuntimeError(f'Error: Cannot create image for {fstype} filesystem in device {padev}')
try:
st = os.stat(image_path)
@@ -519,8 +503,7 @@ class OgLiveOperations:
image_info.mtime = mtime
if self._write_md5_file(f'/opt/opengnsys/images/{name}.img') == -1:
- logging.error(f'cannot write {name}.full.sum file')
- raise ValueError(f'Error: Cannot write {name}.full.sum file')
+ raise ValueError(f'Cannot write {name}.full.sum file')
self._restartBrowser(self._url)