Merge branch 'dev' into enh-python3

debian/changelog

@@ -1,3 +1,29 @@
+moulinette (4.1.4) stable; urgency=low
+
+  - [enh] Remove useless warning
+  - Stable release
+
+  Thanks to all contributors <3 ! (Aleks)
+
+ -- Kayou <pierre@kayou.io>  Thu, 14 Jan 2021 21:49:37 +0100
+
+moulinette (4.1.3) stable; urgency=low
+
+  - Stable release
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Fri, 08 Jan 2021 03:15:05 +0100
+
+moulinette (4.1.2) testing; urgency=low
+
+  - [mod] Misc code cleanup ([#259](https://github.com/YunoHost/moulinette/pull/259), 82bc0e82, 8566eaaa, 2caf1234)
+  - [mod] Improve error messages ([#264](https://github.com/YunoHost/moulinette/pull/264))
+  - [fix] Minor issue about 'comment' in actionmaps ([#266](https://github.com/YunoHost/moulinette/pull/266))
+  - [i18n] Improve translation for French
+
+  Thanks to all contributors <3 ! (Kay0u, Bram, ppr)
+
+ -- Alexandre Aubin <alex.aubin@mailoo.org>  Thu, 07 Jan 2021 00:23:44 +0100
+
 moulinette (4.1.1) testing; urgency=low
 
   - [enh] Add options to write_to_json ([#261](https://github.com/YunoHost/moulinette/pull/261))

locales/en.json

@@ -7,7 +7,6 @@
     "deprecated_command": "'{prog} {command}' is deprecated and will be removed in the future",
     "deprecated_command_alias": "'{prog} {old}' is deprecated and will be removed in the future, use '{prog} {new}' instead",
     "error": "Error:",
-    "error_see_log": "An error occurred. Please see the logs for details, they are located in /var/log/yunohost/.",
     "file_exists": "File already exists: '{path}'",
     "file_not_exist": "File does not exist: '{path}'",
     "folder_exists": "Folder already exists: '{path}'",

locales/fr.json

@@ -56,6 +56,6 @@
     "warn_the_user_about_waiting_lock_again": "Toujours en attente...",
     "warn_the_user_that_lock_is_acquired": "La commande précédente vient de se terminer, lancement de cette nouvelle commande",
     "invalid_token": "Jeton non valide - veuillez vous authentifier",
-    "ldap_server_is_down_restart_it": "Le service LDAP est arrêté, nous tentons de le redémarrer...",
+    "ldap_server_is_down_restart_it": "Le service LDAP s'est arrêté, une tentative de redémarrage est en cours ...",
     "session_expired": "La session a expiré. Merci de vous ré-authentifier."
 }

moulinette/__init__.py

@@ -121,6 +121,7 @@ def cli(args, top_parser, output_as=None, timeout=None):
         )
     except MoulinetteError as e:
         import logging
+
         logging.getLogger(logging.main_logger).error(e.strerror)
         return 1
     return 0

moulinette/actionsmap.py

@@ -110,7 +110,7 @@ class CommentParameter(_ExtraParameter):
     skipped_iface = ["api"]
 
     def __call__(self, message, arg_name, arg_value):
-        if arg_value:
+        if arg_value is None:
             return
         return msignals.display(m18n.n(message))
 
@@ -210,7 +210,7 @@ class PatternParameter(_ExtraParameter):
         # Use temporarly utf-8 encoded value
         try:
             v = str(arg_value, "utf-8")
-        except:
+        except Exception:
             v = arg_value
 
         if v and not re.match(pattern, v or "", re.UNICODE):
@@ -324,14 +324,12 @@ class ExtraArgumentParser(object):
                     # Validate parameter value
                     parameters[p] = klass.validate(parameters[p], arg_name)
                 except Exception as e:
-                    logger.error(
+                    error_message = (
-                        "unable to validate extra parameter '%s' "
+                        "unable to validate extra parameter '%s' for argument '%s': %s"
-                        "for argument '%s': %s",
+                        % (p, arg_name, e)
-                        p,
-                        arg_name,
-                        e,
                     )
-                    raise MoulinetteError("error_see_log")
+                    logger.error(error_message)
+                    raise MoulinetteError(error_message, raw_msg=True)
 
         return parameters
 
@@ -498,10 +496,12 @@ class ActionsMap(object):
         try:
             mod = import_module("moulinette.authenticators.%s" % auth_conf["vendor"])
         except ImportError:
-            logger.exception(
+            error_message = (
-                "unable to load authenticator vendor '%s'", auth_conf["vendor"]
+                "unable to load authenticator vendor module 'moulinette.authenticators.%s'"
+                % auth_conf["vendor"]
             )
-            raise MoulinetteError("error_see_log")
+            logger.exception(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
         else:
             return mod.Authenticator(**auth_conf)
 
@@ -581,12 +581,17 @@ class ActionsMap(object):
                     time() - start,
                 )
                 func = getattr(mod, func_name)
-            except (AttributeError, ImportError):
+            except (AttributeError, ImportError) as e:
                 import traceback
 
                 traceback.print_exc()
-                logger.exception("unable to load function %s.%s", namespace, func_name)
+                error_message = "unable to load function %s.%s because: %s" % (
-                raise MoulinetteError("error_see_log")
+                    namespace,
+                    func_name,
+                    e,
+                )
+                logger.exception(error_message)
+                raise MoulinetteError(error_message, raw_msg=True)
             else:
                 log_id = start_action_logging()
                 if logger.isEnabledFor(logging.DEBUG):

moulinette/authenticators/dummy.py

@@ -11,8 +11,7 @@ logger = logging.getLogger("moulinette.authenticator.dummy")
 
 class Authenticator(BaseAuthenticator):
 
-    """Dummy authenticator used for tests
+    """Dummy authenticator used for tests"""
-    """
 
     vendor = "dummy"
 

moulinette/authenticators/ldap.py

@@ -237,7 +237,7 @@ class Authenticator(BaseAuthenticator):
         ldif = modlist.modifyModlist(actual_entry[0], attr_dict, ignore_oldexistent=1)
 
         if ldif == []:
-            logger.warning("Nothing to update in LDAP")
+            logger.debug("Nothing to update in LDAP")
             return True
 
         try:

moulinette/interfaces/__init__.py

@@ -257,24 +257,24 @@ class BaseActionsMapParser(object):
                     # Store only if authentication is needed
                     conf["authenticate"] = True if self.interface in ifaces else False
             else:
-                logger.error(
+                error_message = (
                     "expecting 'all', 'False' or a list for "
-                    "configuration 'authenticate', got %r",
+                    "configuration 'authenticate', got %r" % ifaces,
-                    ifaces,
                 )
-                raise MoulinetteError("error_see_log")
+                logger.error(error_message)
+                raise MoulinetteError(error_message, raw_msg=True)
 
         # -- 'authenticator'
         auth = configuration.get("authenticator", "default")
         if not is_global and isinstance(auth, str):
             # Store needed authenticator profile
             if auth not in self.global_conf["authenticator"]:
-                logger.error(
+                error_message = (
                     "requesting profile '%s' which is undefined in "
-                    "global configuration of 'authenticator'",
+                    "global configuration of 'authenticator'" % auth,
-                    auth,
                 )
-                raise MoulinetteError("error_see_log")
+                logger.error(error_message)
+                raise MoulinetteError(error_message, raw_msg=True)
             else:
                 conf["authenticator"] = auth
         elif is_global and isinstance(auth, dict):
@@ -293,12 +293,13 @@ class BaseActionsMapParser(object):
                     }
                 conf["authenticator"] = auths
         else:
-            logger.error(
+            error_message = (
                 "expecting a dict of profile(s) or a profile name "
                 "for configuration 'authenticator', got %r",
                 auth,
             )
-            raise MoulinetteError("error_see_log")
+            logger.error(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
 
         return conf
 
@@ -378,12 +379,13 @@ class _CallbackAction(argparse.Action):
         try:
             # Execute callback and get returned value
             value = self.callback(namespace, values, **self.callback_kwargs)
-        except:
+        except Exception as e:
-            logger.exception(
+            error_message = (
                 "cannot get value from callback method "
-                "'{0}'".format(self.callback_method)
+                "'{0}': {1}".format(self.callback_method, e)
             )
-            raise MoulinetteError("error_see_log")
+            logger.exception(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
         else:
             if value:
                 if self.callback_return:

moulinette/interfaces/api.py

@@ -288,7 +288,10 @@ class _ActionsMapPlugin(object):
 
         # Append messages route
         app.route(
-            "/messages", name="messages", callback=self.messages, skip=["actionsmap"],
+            "/messages",
+            name="messages",
+            callback=self.messages,
+            skip=["actionsmap"],
         )
 
         # Append routes from the actions map
@@ -672,9 +675,13 @@ class ActionsMapParser(BaseActionsMapParser):
         try:
             # Retrieve the tid for the route
             tid, _ = self._parsers[kwargs.get("route")]
-        except KeyError:
+        except KeyError as e:
-            logger.error("no argument parser found for route '%s'", kwargs.get("route"))
+            error_message = "no argument parser found for route '%s': %s" % (
-            raise MoulinetteError("error_see_log")
+                kwargs.get("route"),
+                e,
+            )
+            logger.error(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
 
         if self.get_conf(tid, "authenticate"):
             authenticator = self.get_conf(tid, "authenticator")
@@ -700,9 +707,10 @@ class ActionsMapParser(BaseActionsMapParser):
         try:
             # Retrieve the parser for the route
             _, parser = self._parsers[route]
-        except KeyError:
+        except KeyError as e:
-            logger.error("no argument parser found for route '%s'", route)
+            error_message = "no argument parser found for route '%s': %s" % (route, e)
-            raise MoulinetteError("error_see_log")
+            logger.error(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
         ret = argparse.Namespace()
 
         # TODO: Catch errors?
@@ -810,7 +818,9 @@ class Interface(BaseInterface):
 
         """
         logger.debug(
-            "starting the server instance in %s:%d", host, port,
+            "starting the server instance in %s:%d",
+            host,
+            port,
         )
 
         try:
@@ -820,10 +830,15 @@ class Interface(BaseInterface):
             server = WSGIServer((host, port), self._app, handler_class=WebSocketHandler)
             server.serve_forever()
         except IOError as e:
-            logger.exception("unable to start the server instance on %s:%d", host, port)
+            error_message = "unable to start the server instance on %s:%d: %s" % (
+                host,
+                port,
+                e,
+            )
+            logger.exception(error_message)
             if e.args[0] == errno.EADDRINUSE:
                 raise MoulinetteError("server_already_running")
-            raise MoulinetteError("error_see_log")
+            raise MoulinetteError(error_message)
 
     # Routes handlers
 

moulinette/interfaces/cli.py

@@ -376,9 +376,13 @@ class ActionsMapParser(BaseActionsMapParser):
             ret = self._parser.parse_args(args)
         except SystemExit:
             raise
-        except:
+        except Exception as e:
-            logger.exception("unable to parse arguments '%s'", " ".join(args))
+            error_message = "unable to parse arguments '%s' because: %s" % (
-            raise MoulinetteError("error_see_log")
+                " ".join(args),
+                e,
+            )
+            logger.exception(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
 
         tid = getattr(ret, "_tid", None)
         if self.get_conf(tid, "authenticate"):
@@ -400,9 +404,13 @@ class ActionsMapParser(BaseActionsMapParser):
             ret = self._parser.parse_args(args)
         except SystemExit:
             raise
-        except:
+        except Exception as e:
-            logger.exception("unable to parse arguments '%s'", " ".join(args))
+            error_message = "unable to parse arguments '%s' because: %s" % (
-            raise MoulinetteError("error_see_log")
+                " ".join(args),
+                e,
+            )
+            logger.exception(error_message)
+            raise MoulinetteError(error_message, raw_msg=True)
         else:
             self.prepare_action_namespace(getattr(ret, "_tid", None), ret)
             self._parser.dequeue_callbacks(ret)

moulinette/utils/filesystem.py

@@ -158,9 +158,11 @@ def write_to_file(file_path, data, file_mode="w"):
     assert not os.path.isdir(file_path), (
         "Error: file_path '%s' point to a dir, it should be a file" % file_path
     )
-    assert os.path.isdir(os.path.dirname(file_path)), (
+    assert os.path.isdir(
-        "Error: the path ('%s') base dir ('%s') is not a dir"
+        os.path.dirname(file_path)
-        % (file_path, os.path.dirname(file_path))
+    ), "Error: the path ('%s') base dir ('%s') is not a dir" % (
+        file_path,
+        os.path.dirname(file_path),
     )
 
     # If data is a list, check elements are strings and build a single string
@@ -207,16 +209,20 @@ def write_to_json(file_path, data, sort_keys=False, indent=None):
         "Error: file_path '%s' should be a string but is of type '%s' instead"
         % (file_path, type(file_path))
     )
-    assert isinstance(data, dict) or isinstance(data, list), (
+    assert isinstance(data, dict) or isinstance(
-        "Error: data '%s' should be a dict or a list but is of type '%s' instead"
+        data, list
-        % (data, type(data))
+    ), "Error: data '%s' should be a dict or a list but is of type '%s' instead" % (
+        data,
+        type(data),
     )
     assert not os.path.isdir(file_path), (
         "Error: file_path '%s' point to a dir, it should be a file" % file_path
     )
-    assert os.path.isdir(os.path.dirname(file_path)), (
+    assert os.path.isdir(
-        "Error: the path ('%s') base dir ('%s') is not a dir"
+        os.path.dirname(file_path)
-        % (file_path, os.path.dirname(file_path))
+    ), "Error: the path ('%s') base dir ('%s') is not a dir" % (
+        file_path,
+        os.path.dirname(file_path),
     )
 
     # Write dict to file

test/conftest.py

@@ -28,7 +28,7 @@ def patch_translate(moulinette):
 
     def new_translate(self, key, *args, **kwargs):
         if key not in self._translations[self.default_locale].keys():
-            message = "Unable to retrieve key %s for default locale!" % key
+            message = "Unable to retrieve key '%s' for default locale!" % key
             raise KeyError(message)
 
         return old_translate(self, key, *args, **kwargs)

test/src/old_slapdtest/_slapdtest.py

@@ -127,14 +127,12 @@ else:
 
 
 def identity(test_item):
-    """Identity decorator
+    """Identity decorator"""
-    """
     return test_item
 
 
 def skip_unless_ci(reason, feature=None):
-    """Skip test unless test case is executed on CI like Travis CI
+    """Skip test unless test case is executed on CI like Travis CI"""
-    """
     if not os.environ.get("CI", False):
         return unittest.skip(reason)
     elif feature in CI_DISABLED:
@@ -201,7 +199,8 @@ def combined_logger(
             fmt=" ".join((log_name, sys_log_format))
         )
         my_syslog_handler = logging.handlers.SysLogHandler(
-            address="/dev/log", facility=SysLogHandler.LOG_DAEMON,
+            address="/dev/log",
+            facility=SysLogHandler.LOG_DAEMON,
         )
         my_syslog_handler.setFormatter(my_syslog_formatter)
         new_logger.addHandler(my_syslog_handler)
@@ -373,7 +372,8 @@ class SlapdObject(object):
         """
         include_directives = "\n".join(
             'include "{schema_prefix}/{schema_file}"'.format(
-                schema_prefix=self._schema_prefix, schema_file=schema_file,
+                schema_prefix=self._schema_prefix,
+                schema_file=schema_file,
             )
             for schema_file in self.openldap_schema_files
         )
@@ -563,7 +563,13 @@ class SlapdObject(object):
         if ldap_uri is None:
             ldap_uri = self.default_ldap_uri
         args = (
-            [ldapcommand, "-H", ldap_uri,] + self._cli_auth_args() + (extra_args or [])
+            [
+                ldapcommand,
+                "-H",
+                ldap_uri,
+            ]
+            + self._cli_auth_args()
+            + (extra_args or [])
         )
         self._log.debug("Run command: %r", " ".join(args))
         proc = subprocess.Popen(

test/test_actionsmap.py

@@ -188,7 +188,7 @@ def test_extra_argument_parser_add_argument_bad_arg(iface):
     with pytest.raises(MoulinetteError) as exception:
         extra_argument_parse.add_argument(GLOBAL_SECTION, "foo", {"ask": 1})
 
-    translation = m18n.g("error_see_log")
+    translation = m18n.g("error")
     expected_msg = translation.format()
     assert expected_msg in str(exception)
 
@@ -265,7 +265,7 @@ def test_actions_map_import_error(mocker, builtin_str):
     with pytest.raises(MoulinetteError) as exception:
         amap.process({}, timeout=30, route=("GET", "/test-auth/none"))
 
-    translation = m18n.g("error_see_log")
+    translation = m18n.g("error")
     expected_msg = translation.format()
     assert expected_msg in str(exception)
 

test/test_ldap.py

@@ -58,9 +58,11 @@ class TestLDAP:
 
     def test_authenticate_sasl_non_interactive_bind(self, ldap_server):
         self.ldap_conf["parameters"]["uri"] = ldap_server.uri
-        self.ldap_conf["parameters"]["user_rdn"] = (
+        self.ldap_conf["parameters"][
-            "gidNumber=%s+uidNumber=%s,cn=peercred,cn=external,cn=auth"
+            "user_rdn"
-            % (os.getgid(), os.getuid())
+        ] = "gidNumber=%s+uidNumber=%s,cn=peercred,cn=external,cn=auth" % (
+            os.getgid(),
+            os.getuid(),
         )
         ldap_interface = m_ldap.Authenticator(**self.ldap_conf)