From 70d0f8a68f276274bd09cf31788185ed02b480b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Sun, 26 Aug 2018 12:46:42 +0200 Subject: [PATCH 01/15] Add return value in hook_exec --- src/yunohost/hook.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 87844ce17..143742df1 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -280,8 +280,8 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, try: hook_args = pre_callback(name=name, priority=priority, path=path, args=args) - hook_exec(path, args=hook_args, chdir=chdir, env=env, - no_trace=no_trace, raise_on_error=True, user="root") + hook_return = hook_exec(path, args=hook_args, chdir=chdir, env=env, + no_trace=no_trace, raise_on_error=True, user="root")[1] except MoulinetteError as e: state = 'failed' logger.error(e.strerror, exc_info=1) @@ -294,6 +294,10 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, result[state][name].append(path) except KeyError: result[state][name] = [path] + try: + result['stdreturn'].append(hook_return) + except KeyError: + result['stdreturn'] = [hook_return] return result @@ -341,6 +345,11 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, stdinfo = os.path.join(tempfile.mkdtemp(), "stdinfo") env['YNH_STDINFO'] = stdinfo + stdreturn = os.path.join(tempfile.mkdtemp(), "stdreturn") + with open(stdreturn, 'w') as f: + f.write('') + env['YNH_STDRETURN'] = stdreturn + # Construct command to execute if user == "root": command = ['sh', '-c'] @@ -388,11 +397,18 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, errno.EIO, m18n.n('hook_exec_not_terminated', path=path)) else: logger.error(m18n.n('hook_exec_not_terminated', path=path)) - return 1 + return 1, '' elif raise_on_error and returncode != 0: raise MoulinetteError( errno.EIO, m18n.n('hook_exec_failed', path=path)) - return returncode + + with open(stdreturn, 'r') as f: + returnstring = f.read() + stdreturndir = os.path.split(stdreturn)[0] + os.remove(stdreturn) + os.rmdir(stdreturndir) + + return returncode, returnstring def _extract_filename_parts(filename): From b64196c47ddc80926c4d46717da915bb78833a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Sun, 26 Aug 2018 12:31:20 +0200 Subject: [PATCH 02/15] Change return of hook_exec everywhere --- src/yunohost/app.py | 12 ++++++------ src/yunohost/backup.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 1fed09425..bad1797b3 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -513,7 +513,7 @@ def app_change_url(operation_logger, auth, app, domain, path): os.system('chmod +x %s' % os.path.join(os.path.join(APP_TMP_FOLDER, "scripts", "change_url"))) if hook_exec(os.path.join(APP_TMP_FOLDER, 'scripts/change_url'), - args=args_list, env=env_dict, user="root") != 0: + args=args_list, env=env_dict, user="root")[0] != 0: msg = "Failed to change '%s' url." % app logger.error(msg) operation_logger.error(msg) @@ -640,7 +640,7 @@ def app_upgrade(auth, app=[], url=None, file=None): # Execute App upgrade script os.system('chown -hR admin: %s' % INSTALL_TMP) if hook_exec(extracted_app_folder + '/scripts/upgrade', - args=args_list, env=env_dict, user="root") != 0: + args=args_list, env=env_dict, user="root")[0] != 0: msg = m18n.n('app_upgrade_failed', app=app_instance_name) logger.error(msg) operation_logger.error(msg) @@ -801,7 +801,7 @@ def app_install(operation_logger, auth, app, label=None, args=None, no_remove_on install_retcode = hook_exec( os.path.join(extracted_app_folder, 'scripts/install'), args=args_list, env=env_dict, user="root" - ) + )[0] except (KeyboardInterrupt, EOFError): install_retcode = -1 except: @@ -825,7 +825,7 @@ def app_install(operation_logger, auth, app, label=None, args=None, no_remove_on remove_retcode = hook_exec( os.path.join(extracted_app_folder, 'scripts/remove'), args=[app_instance_name], env=env_dict_remove, user="root" - ) + )[0] if remove_retcode != 0: msg = m18n.n('app_not_properly_removed', app=app_instance_name) @@ -912,7 +912,7 @@ def app_remove(operation_logger, auth, app): operation_logger.flush() if hook_exec('/tmp/yunohost_remove/scripts/remove', args=args_list, - env=env_dict, user="root") == 0: + env=env_dict, user="root")[0] == 0: logger.success(m18n.n('app_removed', app=app)) hook_callback('post_app_remove', args=args_list, env=env_dict) @@ -1518,7 +1518,7 @@ def app_action_run(app_id, action, args=None): env=env_dict, chdir=cwd, user=action_declaration.get("user", "root"), - ) + )[0] if retcode not in action_declaration.get("accepted_return_codes", [0]): raise MoulinetteError(retcode, "Error while executing action '%s' of app '%s': return code %s" % (action, app_id, retcode)) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 88959cc2f..bad2f53c2 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -680,7 +680,7 @@ class BackupManager(): subprocess.call(['install', '-Dm555', app_script, tmp_script]) hook_exec(tmp_script, args=[tmp_app_bkp_dir, app], - raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict, user="root") + raise_on_error=True, chdir=tmp_app_bkp_dir, env=env_dict, user="root")[0] self._import_to_list_to_backup(env_dict["YNH_BACKUP_CSV"]) except: @@ -1311,7 +1311,7 @@ class RestoreManager(): chdir=app_backup_in_archive, raise_on_error=True, env=env_dict, - user="root") + user="root")[0] except: msg = m18n.n('restore_app_failed',app=app_instance_name) logger.exception(msg) @@ -1336,7 +1336,7 @@ class RestoreManager(): # Execute remove script # TODO: call app_remove instead if hook_exec(remove_script, args=[app_instance_name], - env=env_dict_remove, user="root") != 0: + env=env_dict_remove, user="root")[0] != 0: msg = m18n.n('app_not_properly_removed', app=app_instance_name) logger.warning(msg) operation_logger.error(msg) From b7554dec2176f38df20a0d49a42d522a7c702ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Sun, 26 Aug 2018 14:59:26 +0200 Subject: [PATCH 03/15] Use json for return --- locales/en.json | 1 + src/yunohost/hook.py | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/locales/en.json b/locales/en.json index 074512311..1e7e78490 100644 --- a/locales/en.json +++ b/locales/en.json @@ -199,6 +199,7 @@ "global_settings_unknown_type": "Unexpected situation, the setting {setting:s} appears to have the type {unknown_type:s} but it's not a type supported by the system.", "hook_exec_failed": "Script execution failed: {path:s}", "hook_exec_not_terminated": "Script execution hasn\u2019t terminated: {path:s}", + "hook_json_return_error": "Faild to read return from hook {path:s}. Error: {msg:s}", "hook_list_by_invalid": "Invalid property to list hook by", "hook_name_unknown": "Unknown hook name '{name:s}'", "installation_complete": "Installation complete", diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 143742df1..3ef05980a 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -32,6 +32,7 @@ from glob import iglob from moulinette import m18n from moulinette.core import MoulinetteError from moulinette.utils import log +from moulinette.utils.filesystem import read_json HOOK_FOLDER = '/usr/share/yunohost/hooks/' CUSTOM_HOOK_FOLDER = '/etc/yunohost/hooks.d/' @@ -229,7 +230,7 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, (name, priority, path, succeed) as arguments """ - result = {'succeed': {}, 'failed': {}} + result = {'succeed': {}, 'failed': {}, 'stdreturn' : []} hooks_dict = {} # Retrieve hooks @@ -294,10 +295,13 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, result[state][name].append(path) except KeyError: result[state][name] = [path] - try: - result['stdreturn'].append(hook_return) - except KeyError: - result['stdreturn'] = [hook_return] + + #print(hook_return) + #for r in hook_return.: + result['stdreturn'].extend(hook_return) #for r in hook_return + #print(r) + + #print(result['stdreturn']) return result @@ -402,13 +406,17 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, raise MoulinetteError( errno.EIO, m18n.n('hook_exec_failed', path=path)) - with open(stdreturn, 'r') as f: - returnstring = f.read() + try: + returnjson = read_json(stdreturn) + except Exception as e: + returnjson = {} + errno.EIO, m18n.n('hook_json_return_error', path=path, msg=str(e)) + stdreturndir = os.path.split(stdreturn)[0] os.remove(stdreturn) os.rmdir(stdreturndir) - return returncode, returnstring + return returncode, returnjson def _extract_filename_parts(filename): From 29bf70c57cd7ec24ac33d5688ccd75b2ac5bd101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Wed, 29 Aug 2018 18:54:39 +0200 Subject: [PATCH 04/15] Add hook name in return structure in hook_callback --- src/yunohost/hook.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 3ef05980a..efd14ca75 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -230,7 +230,6 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, (name, priority, path, succeed) as arguments """ - result = {'succeed': {}, 'failed': {}, 'stdreturn' : []} hooks_dict = {} # Retrieve hooks @@ -292,16 +291,11 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, post_callback(name=name, priority=priority, path=path, succeed=True) try: - result[state][name].append(path) + result[name][state].append(path) except KeyError: - result[state][name] = [path] + result[name][state] = [path] - #print(hook_return) - #for r in hook_return.: - result['stdreturn'].extend(hook_return) #for r in hook_return - #print(r) - - #print(result['stdreturn']) + result[name]['stdreturn'] = hook_return return result From e4e981c0fe4e635001619e3bd339aa5c32834954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Wed, 29 Aug 2018 20:56:00 +0200 Subject: [PATCH 05/15] Change struct returned by hook_callback --- src/yunohost/hook.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index efd14ca75..ce83ce011 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -230,6 +230,7 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, (name, priority, path, succeed) as arguments """ + result = {} hooks_dict = {} # Retrieve hooks @@ -290,12 +291,8 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, else: post_callback(name=name, priority=priority, path=path, succeed=True) - try: - result[name][state].append(path) - except KeyError: - result[name][state] = [path] - result[name]['stdreturn'] = hook_return + result[name] = {'path' : path, 'state' : state, 'stdreturn' : hook_return } return result From 8ec6f2b81a2ee91e450a3a49d7fa627713d61020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Thu, 30 Aug 2018 17:37:28 +0200 Subject: [PATCH 06/15] Fix error if hook return nothing --- src/yunohost/hook.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index ce83ce011..7ad95c8ab 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -398,10 +398,15 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, errno.EIO, m18n.n('hook_exec_failed', path=path)) try: - returnjson = read_json(stdreturn) + with open(stdreturn, 'r') as f: + if f.read() != '': + returnjson = read_json(stdreturn) + else: + returnjson = {} except Exception as e: returnjson = {} - errno.EIO, m18n.n('hook_json_return_error', path=path, msg=str(e)) + raise MoulinetteError( + errno.EIO, m18n.n('hook_json_return_error', path=path, msg=str(e))) stdreturndir = os.path.split(stdreturn)[0] os.remove(stdreturn) From 8012f12797e7025f6832db6fa60e60db3c8c247a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Wed, 5 Dec 2018 00:32:42 +0100 Subject: [PATCH 07/15] Fix raise error --- src/yunohost/hook.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 121630e9f..4e219d226 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -404,7 +404,8 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, else: returnjson = {} except Exception as e: - returnjson = {} + os.remove(stdreturn) + os.rmdir(stdreturndir) raise MoulinetteError( errno.EIO, m18n.n('hook_json_return_error', path=path, msg=str(e))) From 6ab5d716037b76a913c30a43b6ad1e2d57870428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Mon, 11 Feb 2019 20:05:21 +0100 Subject: [PATCH 08/15] Add the possibility to have multiple path per hook name --- src/yunohost/hook.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 02e3cb2dd..d9cad9c7a 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -283,14 +283,16 @@ def hook_callback(action, hooks=[], args=None, no_trace=False, chdir=None, no_trace=no_trace, raise_on_error=True)[1] except YunohostError as e: state = 'failed' + hook_return = {} logger.error(e.strerror, exc_info=1) post_callback(name=name, priority=priority, path=path, succeed=False) else: post_callback(name=name, priority=priority, path=path, succeed=True) - - result[name] = {'path' : path, 'state' : state, 'stdreturn' : hook_return } + if not name in result: + result[name] = {} + result[name][path] = {'state' : state, 'stdreturn' : hook_return } return result @@ -389,7 +391,7 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, raise YunohostError('hook_exec_not_terminated', path=path) else: logger.error(m18n.n('hook_exec_not_terminated', path=path)) - return 1, '' + return 1, {} elif raise_on_error and returncode != 0: raise YunohostError('hook_exec_failed', path=path) From 37a2cc2e1c155f420f283d05bb0eecdf7ad6d1a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josu=C3=A9=20Tille?= Date: Mon, 11 Feb 2019 20:05:50 +0100 Subject: [PATCH 09/15] Adapt the service and backup to support new result of hook_callback result --- src/yunohost/backup.py | 30 +++++++++++++++++++----------- src/yunohost/service.py | 5 +++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 3874a4461..d33f75e3a 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -593,8 +593,11 @@ class BackupManager(): env=env_dict, chdir=self.work_dir) - if ret["succeed"] != []: - self.system_return = ret["succeed"] + ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} + ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + + if ret_succeed != []: + self.system_return = ret_succeed # Add files from targets (which they put in the CSV) to the list of # files to backup @@ -610,7 +613,7 @@ class BackupManager(): restore_hooks = hook_list("restore")["hooks"] - for part in ret['succeed'].keys(): + for part in ret_succeed.keys(): if part in restore_hooks: part_restore_hooks = hook_info("restore", part)["hooks"] for hook in part_restore_hooks: @@ -620,7 +623,7 @@ class BackupManager(): logger.warning(m18n.n('restore_hook_unavailable', hook=part)) self.targets.set_result("system", part, "Warning") - for part in ret['failed'].keys(): + for part in ret_failed.keys(): logger.error(m18n.n('backup_system_part_failed', part=part)) self.targets.set_result("system", part, "Error") @@ -1177,16 +1180,19 @@ class RestoreManager(): env=env_dict, chdir=self.work_dir) - for part in ret['succeed'].keys(): + ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} + ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + + for part in ret_succeed.keys(): self.targets.set_result("system", part, "Success") error_part = [] - for part in ret['failed'].keys(): + for part in ret_failed.keys(): logger.error(m18n.n('restore_system_part_failed', part=part)) self.targets.set_result("system", part, "Error") error_part.append(part) - if ret['failed']: + if ret_failed: operation_logger.error(m18n.n('restore_system_part_failed', part=', '.join(error_part))) else: operation_logger.success() @@ -1929,8 +1935,8 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('need_mount')) - - self._need_mount = True if ret['succeed'] else False + ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} + self._need_mount = True if ret_succeed else False return self._need_mount def backup(self): @@ -1943,7 +1949,8 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('backup')) - if ret['failed']: + ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + if ret_failed: raise YunohostError('backup_custom_backup_error') def mount(self, restore_manager): @@ -1956,7 +1963,8 @@ class CustomBackupMethod(BackupMethod): super(CustomBackupMethod, self).mount(restore_manager) ret = hook_callback('backup_method', [self.method], args=self._get_args('mount')) - if ret['failed']: + ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + if ret_failed: raise YunohostError('backup_custom_mount_error') def _get_args(self, action): diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 60729053b..151a877b9 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -494,11 +494,12 @@ def service_regen_conf(operation_logger, names=[], with_diff=False, force=False, pre_result = hook_callback('conf_regen', names, pre_callback=_pre_call) # Update the services name - names = pre_result['succeed'].keys() + names = {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in pre_result.items()}.keys() if not names: + ret_failed = {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in pre_result.items()} raise YunohostError('service_regenconf_failed', - services=', '.join(pre_result['failed'])) + services=', '.join(ret_failed)) # Set the processing method _regen = _process_regen_conf if not dry_run else lambda *a, **k: True From c191572a255e513f0b511873ebfb9ce04d58407e Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 25 Feb 2019 01:23:05 +0100 Subject: [PATCH 10/15] Typo --- locales/en.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index ac9091c19..a6d95ada1 100644 --- a/locales/en.json +++ b/locales/en.json @@ -211,7 +211,7 @@ "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).", "hook_exec_failed": "Script execution failed: {path:s}", "hook_exec_not_terminated": "Script execution hasn\u2019t terminated: {path:s}", - "hook_json_return_error": "Faild to read return from hook {path:s}. Error: {msg:s}", + "hook_json_return_error": "Failed to read return from hook {path:s}. Error: {msg:s}", "hook_list_by_invalid": "Invalid property to list hook by", "hook_name_unknown": "Unknown hook name '{name:s}'", "installation_complete": "Installation complete", From 877fe506850b388807cfebf0e9d7561776ac2637 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Mar 2019 17:21:57 +0100 Subject: [PATCH 11/15] Use a finally clause to avoid duplicated code --- src/yunohost/hook.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index d9cad9c7a..961caf606 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -403,13 +403,11 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, else: returnjson = {} except Exception as e: + raise YunohostError('hook_json_return_error', path=path, msg=str(e)) + finally: + stdreturndir = os.path.split(stdreturn)[0] os.remove(stdreturn) os.rmdir(stdreturndir) - raise YunohostError('hook_json_return_error', path=path, msg=str(e)) - - stdreturndir = os.path.split(stdreturn)[0] - os.remove(stdreturn) - os.rmdir(stdreturndir) return returncode, returnjson From c74bff31d12e08acc80f54f628e09ce8c16f7adc Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Mar 2019 17:44:20 +0100 Subject: [PATCH 12/15] Fix listing of succeed/failed hook names --- src/yunohost/service.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/yunohost/service.py b/src/yunohost/service.py index 151a877b9..61274aaac 100644 --- a/src/yunohost/service.py +++ b/src/yunohost/service.py @@ -493,11 +493,14 @@ def service_regen_conf(operation_logger, names=[], with_diff=False, force=False, pre_result = hook_callback('conf_regen', names, pre_callback=_pre_call) - # Update the services name - names = {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in pre_result.items()}.keys() + # Keep only the hook names with at least one success + names = [hook for hook, infos in pre_result.items() + if any(result["state"] == "succeed" for result in infos.values())] + # FIXME : what do in case of partial success/failure ... if not names: - ret_failed = {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in pre_result.items()} + ret_failed = [hook for hook, infos in pre_result.items() + if any(result["state"] == "failed" for result in infos.values())] raise YunohostError('service_regenconf_failed', services=', '.join(ret_failed)) From 16245f53d9591fcfeca70e5c54604ba3d26c2185 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Mar 2019 18:33:09 +0100 Subject: [PATCH 13/15] Fix listing of succeed/failed for backup and restore --- src/yunohost/backup.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index d33f75e3a..872896ecb 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -593,10 +593,14 @@ class BackupManager(): env=env_dict, chdir=self.work_dir) - ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} - ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + ret_succeed = {hook: {path:result["state"] for path, result in infos.items()} + for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())} + ret_failed = {hook: {path:result["state"] for path, result in infos.items.items()} + for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())} - if ret_succeed != []: + if ret_succeed.keys() != []: self.system_return = ret_succeed # Add files from targets (which they put in the CSV) to the list of @@ -1180,14 +1184,16 @@ class RestoreManager(): env=env_dict, chdir=self.work_dir) - ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} - ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + ret_succeed = [hook for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())] + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] - for part in ret_succeed.keys(): + for part in ret_succeed: self.targets.set_result("system", part, "Success") error_part = [] - for part in ret_failed.keys(): + for part in ret_failed: logger.error(m18n.n('restore_system_part_failed', part=part)) self.targets.set_result("system", part, "Error") error_part.append(part) From 88b7d77610d6493d62a490fa3ff758a992e7e270 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Mar 2019 18:58:04 +0100 Subject: [PATCH 14/15] Display raw json content in case of exception --- locales/en.json | 2 +- src/yunohost/hook.py | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/locales/en.json b/locales/en.json index a6d95ada1..ca1d9c8a1 100644 --- a/locales/en.json +++ b/locales/en.json @@ -211,7 +211,7 @@ "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).", "hook_exec_failed": "Script execution failed: {path:s}", "hook_exec_not_terminated": "Script execution hasn\u2019t terminated: {path:s}", - "hook_json_return_error": "Failed to read return from hook {path:s}. Error: {msg:s}", + "hook_json_return_error": "Failed to read return from hook {path:s}. Error: {msg:s}. Raw content: {raw_content}", "hook_list_by_invalid": "Invalid property to list hook by", "hook_name_unknown": "Unknown hook name '{name:s}'", "installation_complete": "Installation complete", diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index 961caf606..c4605b6e8 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -395,15 +395,17 @@ def hook_exec(path, args=None, raise_on_error=False, no_trace=False, elif raise_on_error and returncode != 0: raise YunohostError('hook_exec_failed', path=path) + raw_content = None try: - - with open(stdreturn, 'r') as f: - if f.read() != '': - returnjson = read_json(stdreturn) - else: - returnjson = {} + with open(stdreturn, 'r') as f: + raw_content = f.read() + if raw_content != '': + returnjson = read_json(stdreturn) + else: + returnjson = {} except Exception as e: - raise YunohostError('hook_json_return_error', path=path, msg=str(e)) + raise YunohostError('hook_json_return_error', path=path, msg=str(e), + raw_content=raw_content) finally: stdreturndir = os.path.split(stdreturn)[0] os.remove(stdreturn) From 5516e96d076eba0fc98f714b73e365693ddfc7a2 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Mar 2019 19:05:43 +0100 Subject: [PATCH 15/15] Simpler fetching of failed/succeeded hooks in CustomBackupMethod --- src/yunohost/backup.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index 872896ecb..867de3ea9 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1941,7 +1941,8 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('need_mount')) - ret_succeed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "succeed"] for n, v in ret.items()}.items() if val} + ret_succeed = [hook for hook, infos in ret.items() + if any(result["state"] == "succeed" for result in infos.values())] self._need_mount = True if ret_succeed else False return self._need_mount @@ -1955,7 +1956,9 @@ class CustomBackupMethod(BackupMethod): ret = hook_callback('backup_method', [self.method], args=self._get_args('backup')) - ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] if ret_failed: raise YunohostError('backup_custom_backup_error') @@ -1969,7 +1972,9 @@ class CustomBackupMethod(BackupMethod): super(CustomBackupMethod, self).mount(restore_manager) ret = hook_callback('backup_method', [self.method], args=self._get_args('mount')) - ret_failed = {k: val for k, val in {n: [p for p, c in v.items() if c['state'] == "failed"] for n, v in ret.items()}.items() if val} + + ret_failed = [hook for hook, infos in ret.items() + if any(result["state"] == "failed" for result in infos.values())] if ret_failed: raise YunohostError('backup_custom_mount_error')