Merge pull request #327 from YunoHost/dont-take-lock-for-read-operations

Dont take lock for read/GET operations

moulinette/actionsmap.py

@@ -515,6 +515,8 @@ class ActionsMap:
         tid = arguments.pop("_tid")
         arguments = self.extraparser.parse_args(tid, arguments)
 
+        want_to_take_lock = self.parser.want_to_take_lock(args, **kwargs)
+
         # Retrieve action information
         if len(tid) == 4:
             namespace, category, subcategory, action = tid
@@ -537,7 +539,7 @@ class ActionsMap:
             full_action_name = "{}.{}.{}".format(namespace, category, action)
 
         # Lock the moulinette for the namespace
-        with MoulinetteLock(namespace, timeout, self.enable_lock):
+        with MoulinetteLock(namespace, timeout, self.enable_lock and want_to_take_lock):
             start = time()
             try:
                 mod = __import__(
@@ -662,6 +664,14 @@ class ActionsMap:
                 if interface_type in authentication:
                     action_parser.authentication = authentication[interface_type]
 
+                # Disable the locking mechanism for all actions that are 'GET' actions on the api
+                routes = action_options.get("api")
+                routes = [routes] if isinstance(routes, str) else routes
+                if routes and all(route.startswith("GET ") for route in routes):
+                    action_parser.want_to_take_lock = False
+                else:
+                    action_parser.want_to_take_lock = True
+
             # subcategory_name is like "cert" in "domain cert status"
             # subcategory_values is the values of this subcategory (like actions)
             for subcategory_name, subcategory_values in subcategories.items():
@@ -705,5 +715,13 @@ class ActionsMap:
                     if interface_type in authentication:
                         action_parser.authentication = authentication[interface_type]
 
+                    # Disable the locking mechanism for all actions that are 'GET' actions on the api
+                    routes = action_options.get("api")
+                    routes = [routes] if isinstance(routes, str) else routes
+                    if routes and all(route.startswith("GET ") for route in routes):
+                        action_parser.want_to_take_lock = False
+                    else:
+                        action_parser.want_to_take_lock = True
+
         logger.debug("building parser took %.3fs", time() - start)
         return top_parser

moulinette/interfaces/__init__.py

@@ -5,12 +5,11 @@ import logging
 import argparse
 import copy
 import datetime
-from collections import deque, OrderedDict
+from collections import OrderedDict
 from json.encoder import JSONEncoder
 from typing import Optional
 
 from moulinette import m18n
-from moulinette.core import MoulinetteError
 
 logger = logging.getLogger("moulinette.interface")
 

moulinette/interfaces/api.py

@@ -647,6 +647,12 @@ class ActionsMapParser(BaseActionsMapParser):
 
         return parser.authentication
 
+    def want_to_take_lock(self, _, route):
+
+        _, parser = self._parsers[route]
+
+        return getattr(parser, "want_to_take_lock", True)
+
     def parse_args(self, args, **kwargs):
         """Parse arguments
 

moulinette/interfaces/cli.py

@@ -397,25 +397,9 @@ class ActionsMapParser(BaseActionsMapParser):
         )
 
     def auth_method(self, args):
-        # FIXME? idk .. this try/except is duplicated from parse_args below
-        # Just to be able to obtain the tid
-        try:
-            ret = self._parser.parse_args(args)
-        except SystemExit:
-            raise
-        except Exception as e:
-            error_message = "unable to parse arguments '{}' because: {}".format(
-                " ".join(args),
-                e,
-            )
-            logger.exception(error_message)
-            raise MoulinetteValidationError(error_message, raw_msg=True)
 
-        tid = getattr(ret, "_tid", None)
-
-        # Ugh that's for yunohost --version ...
-        if tid is None:
-            return None
+        ret = self.parse_args(args)
+        tid = getattr(ret, "_tid", [])
 
         # We go down in the subparser tree until we find the leaf
         # corresponding to the tid with a defined authentication
@@ -443,6 +427,17 @@ class ActionsMapParser(BaseActionsMapParser):
             logger.exception(error_message)
             raise MoulinetteValidationError(error_message, raw_msg=True)
 
+    def want_to_take_lock(self, args):
+
+        ret = self.parse_args(args)
+        tid = getattr(ret, "_tid", [])
+        if len(tid) == 3:
+            _p = self._subparsers.choices[tid[1]]._actions[1].choices[tid[2]]
+        elif len(tid) == 4:
+            _p = self._subparsers.choices[tid[1]]._actions[1].choices[tid[2]]._actions[1].choices[tid[3]]
+
+        return getattr(_p, "want_to_take_lock", True)
+
 
 class Interface:
 

setup.cfg

@@ -4,5 +4,7 @@ ignore =
   E128,
   E731,
   E722,
+  # Black formatter conflict
   W503,
-  E203
+  # Black formatter conflict
+  E203