From 676a19dc283b6acced92f71a3494a99b7ee946b5 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 24 Dec 2022 00:02:24 +0100 Subject: [PATCH 1/3] Don't take lock for read/GET operations --- moulinette/actionsmap.py | 18 +++++++++++++++++- moulinette/interfaces/api.py | 6 ++++++ moulinette/interfaces/cli.py | 31 +++++++++++++------------------ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index 3a2bf2b9..c9da16b5 100644 --- a/moulinette/actionsmap.py +++ b/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,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") + if routes and isinstance(routes, str) and routes.startswith("GET "): + 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 +714,12 @@ 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") + if routes and isinstance(routes, str) and routes.startswith("GET "): + 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 diff --git a/moulinette/interfaces/api.py b/moulinette/interfaces/api.py index 20c039b4..9007d773 100644 --- a/moulinette/interfaces/api.py +++ b/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 diff --git a/moulinette/interfaces/cli.py b/moulinette/interfaces/cli.py index efb37aba..b6846b0a 100644 --- a/moulinette/interfaces/cli.py +++ b/moulinette/interfaces/cli.py @@ -387,25 +387,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 @@ -433,6 +417,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: From 1e79e99cc868d73efd823db06b02e7f7dfe9fa2f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 24 Dec 2022 00:57:17 +0100 Subject: [PATCH 2/3] Don't take lock for read/GET operations : also cover cases when there's a list of routes --- moulinette/actionsmap.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/moulinette/actionsmap.py b/moulinette/actionsmap.py index c9da16b5..7e51deb0 100644 --- a/moulinette/actionsmap.py +++ b/moulinette/actionsmap.py @@ -666,7 +666,8 @@ class ActionsMap: # Disable the locking mechanism for all actions that are 'GET' actions on the api routes = action_options.get("api") - if routes and isinstance(routes, str) and routes.startswith("GET "): + 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 @@ -716,7 +717,8 @@ class ActionsMap: # Disable the locking mechanism for all actions that are 'GET' actions on the api routes = action_options.get("api") - if routes and isinstance(routes, str) and routes.startswith("GET "): + 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 From baa00c08123b37bdb2af07d8af73494a110499ec Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 24 Dec 2022 19:28:26 +0100 Subject: [PATCH 3/3] Fix flake8 --- moulinette/interfaces/__init__.py | 3 +-- setup.cfg | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/moulinette/interfaces/__init__.py b/moulinette/interfaces/__init__.py index d3e4e837..d816b5c7 100644 --- a/moulinette/interfaces/__init__.py +++ b/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") diff --git a/setup.cfg b/setup.cfg index 71be731f..1d1ea6e0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,5 +4,7 @@ ignore = E128, E731, E722, - W503 # Black formatter conflict - E203 # Black formatter conflict + # Black formatter conflict + W503, + # Black formatter conflict + E203