From bdef7715f943a635ac26cf70fde7b0cece87db3f Mon Sep 17 00:00:00 2001 From: Christophe Haen Date: Thu, 23 Jun 2022 16:24:01 +0200 Subject: [PATCH 1/7] feat (Core): Add a type hint for DIRAC structure --- src/DIRAC/Core/Utilities/ReturnValues.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/DIRAC/Core/Utilities/ReturnValues.py b/src/DIRAC/Core/Utilities/ReturnValues.py index ff90e656fde..477c09bfacc 100755 --- a/src/DIRAC/Core/Utilities/ReturnValues.py +++ b/src/DIRAC/Core/Utilities/ReturnValues.py @@ -8,10 +8,21 @@ import functools import sys import traceback +from typing import TypedDict, Any, Optional as Opt from DIRAC.Core.Utilities.DErrno import strerror +class DReturnType(TypedDict): + """used for typing the DIRAC return structure""" + + OK: bool + Value: Opt[Any] + Message: Opt[str] + ExecInfo: Opt[tuple] + CallStack: Opt[list[str]] + + def S_ERROR(*args, **kwargs): """return value on error condition From 6e60a38479f32e09f2f8065637c0a491460f1ff3 Mon Sep 17 00:00:00 2001 From: Christophe Haen Date: Thu, 11 Aug 2022 13:41:26 +0200 Subject: [PATCH 2/7] feat (docs): annotate convertToReturnValue --- pyproject.toml | 4 ++++ src/DIRAC/Core/Utilities/ReturnValues.py | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c946fe90262..295548bd932 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,3 +11,7 @@ git_describe_command = "git describe --dirty --tags --long --match *[0-9].[0-9]* [tool.black] line-length = 120 target-version = ['py39'] + +[tool.pylint.typecheck] +# List of decorators that change the signature of a decorated function. +signature-mutators = [] diff --git a/src/DIRAC/Core/Utilities/ReturnValues.py b/src/DIRAC/Core/Utilities/ReturnValues.py index 477c09bfacc..a4036f61ba3 100755 --- a/src/DIRAC/Core/Utilities/ReturnValues.py +++ b/src/DIRAC/Core/Utilities/ReturnValues.py @@ -8,7 +8,7 @@ import functools import sys import traceback -from typing import TypedDict, Any, Optional as Opt +from typing import TypedDict, Any, Optional as Opt, Callable from DIRAC.Core.Utilities.DErrno import strerror @@ -195,7 +195,7 @@ def returnValueOrRaise(result): return result["Value"] -def convertToReturnValue(func): +def convertToReturnValue(func: Callable[..., Any]) -> Callable[..., DReturnType]: """Decorate a function to convert return values to `S_OK`/`S_ERROR` If `func` returns, wrap the return value in `S_OK`. @@ -220,4 +220,7 @@ def wrapped(*args, **kwargs): retval["CallStack"] = traceback.format_tb(exc_tb) return retval + # functools will copy the annotations. Since we change the return type + # we have to update it + wrapped.__annotations__["return"] = DReturnType return wrapped From ecd12f76b0c75cd5de1448324055cb8e1f33e390 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Mon, 15 Aug 2022 07:51:39 +0200 Subject: [PATCH 3/7] test (mypy): Add mypy configuration to pyproject.toml --- pyproject.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 295548bd932..695038f7fd6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,3 +15,12 @@ target-version = ['py39'] [tool.pylint.typecheck] # List of decorators that change the signature of a decorated function. signature-mutators = [] + +[tool.mypy] +allow_redefinition = true +strict = true +check_untyped_defs = true +ignore_missing_imports = true +exclude = [ + '/tests/' +] From d458ae037097150387cb52b393b8842c7f4fbd71 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Mon, 15 Aug 2022 07:59:16 +0200 Subject: [PATCH 4/7] test (typing): Add typing to ReturnValues.py --- src/DIRAC/Core/Utilities/DErrno.py | 2 +- src/DIRAC/Core/Utilities/ReturnValues.py | 85 ++++++++++++++++-------- 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/src/DIRAC/Core/Utilities/DErrno.py b/src/DIRAC/Core/Utilities/DErrno.py index ed028cf3739..54bb7797e5c 100644 --- a/src/DIRAC/Core/Utilities/DErrno.py +++ b/src/DIRAC/Core/Utilities/DErrno.py @@ -297,7 +297,7 @@ } -def strerror(code): +def strerror(code: int) -> str: """This method wraps up os.strerror, and behave the same way. It completes it with the DIRAC specific errors. """ diff --git a/src/DIRAC/Core/Utilities/ReturnValues.py b/src/DIRAC/Core/Utilities/ReturnValues.py index a4036f61ba3..61f859ed16f 100755 --- a/src/DIRAC/Core/Utilities/ReturnValues.py +++ b/src/DIRAC/Core/Utilities/ReturnValues.py @@ -5,25 +5,45 @@ keys are converted to string """ +from __future__ import annotations + import functools import sys import traceback -from typing import TypedDict, Any, Optional as Opt, Callable +from types import TracebackType +from typing import Any, Callable, cast, Generic, Literal, overload, Type, TypeVar, Union +from typing_extensions import TypedDict, ParamSpec, NotRequired from DIRAC.Core.Utilities.DErrno import strerror -class DReturnType(TypedDict): +T = TypeVar("T") +P = ParamSpec("P") + + +class DOKReturnType(TypedDict, Generic[T]): + """used for typing the DIRAC return structure""" + + OK: Literal[True] + Value: T + + +class DErrorReturnType(TypedDict): """used for typing the DIRAC return structure""" - OK: bool - Value: Opt[Any] - Message: Opt[str] - ExecInfo: Opt[tuple] - CallStack: Opt[list[str]] + OK: Literal[False] + Message: str + Errno: int + ExecInfo: NotRequired[ + Union[tuple[None, None, None], tuple[Type[BaseException], BaseException, TracebackType]], + ] + CallStack: NotRequired[list[str]] + +DReturnType = Union[DOKReturnType[T], DErrorReturnType] -def S_ERROR(*args, **kwargs): + +def S_ERROR(*args: Any, **kwargs: Any) -> DErrorReturnType: """return value on error condition Arguments are either Errno and ErrorMessage or just ErrorMessage fro backward compatibility @@ -34,7 +54,7 @@ def S_ERROR(*args, **kwargs): """ callStack = kwargs.pop("callStack", None) - result = {"OK": False, "Errno": 0, "Message": ""} + result: DErrorReturnType = {"OK": False, "Errno": 0, "Message": ""} message = "" if args: @@ -58,14 +78,21 @@ def S_ERROR(*args, **kwargs): result["CallStack"] = callStack - # print "AT >>> S_ERROR", result['OK'], result['Errno'], result['Message'] - # for item in result['CallStack']: - # print item - return result -def S_OK(value=None): +# mypy doesn't understand default parameter values with generics so use overloads (python/mypy#3737) +@overload +def S_OK() -> DOKReturnType[None]: + ... + + +@overload +def S_OK(value: T) -> DOKReturnType[T]: + ... + + +def S_OK(value=None): # type: ignore """return value on success :param value: value of the 'Value' @@ -74,7 +101,7 @@ def S_OK(value=None): return {"OK": True, "Value": value} -def isReturnStructure(unk): +def isReturnStructure(unk: Any) -> bool: """Check if value is an `S_OK`/`S_ERROR` object""" if not isinstance(unk, dict): return False @@ -86,7 +113,7 @@ def isReturnStructure(unk): return "Message" in unk -def isSError(value): +def isSError(value: Any) -> bool: """Check if value is an `S_ERROR` object""" if not isinstance(value, dict): return False @@ -95,7 +122,7 @@ def isSError(value): return "Message" in value -def reprReturnErrorStructure(struct, full=False): +def reprReturnErrorStructure(struct: DErrorReturnType, full: bool = False) -> str: errorNumber = struct.get("Errno", 0) message = struct.get("Message", "") if errorNumber: @@ -111,7 +138,7 @@ def reprReturnErrorStructure(struct, full=False): return reprStr -def returnSingleResult(dictRes): +def returnSingleResult(dictRes: DReturnType[Any]) -> DReturnType[Any]: """Transform the S_OK{Successful/Failed} dictionary convention into an S_OK/S_ERROR return. To be used when a single returned entity is expected from a generally bulk call. @@ -147,7 +174,7 @@ def returnSingleResult(dictRes): errorMessage = list(dictRes["Value"]["Failed"].values())[0] if isinstance(errorMessage, dict): if isReturnStructure(errorMessage): - return errorMessage + return cast(DErrorReturnType, errorMessage) else: return S_ERROR(str(errorMessage)) return S_ERROR(errorMessage) @@ -161,7 +188,7 @@ def returnSingleResult(dictRes): class SErrorException(Exception): """Exception class for use with `convertToReturnValue`""" - def __init__(self, result): + def __init__(self, result: Union[DErrorReturnType, str]): """Create a new exception return value If `result` is a `S_ERROR` return it directly else convert it to an @@ -171,10 +198,10 @@ def __init__(self, result): """ if not isSError(result): result = S_ERROR(result) - self.result = result + self.result = cast(DErrorReturnType, result) -def returnValueOrRaise(result): +def returnValueOrRaise(result: DReturnType[T]) -> T: """Unwrap an S_OK/S_ERROR response into a value or Exception This method assists with using exceptions in DIRAC code by raising @@ -188,14 +215,14 @@ def returnValueOrRaise(result): If no exception is known an :exc:`SErrorException` is raised. """ if not result["OK"]: - if "ExecInfo" in result: + if "ExecInfo" in result and result["ExecInfo"][0]: raise result["ExecInfo"][0] else: raise SErrorException(result) return result["Value"] -def convertToReturnValue(func: Callable[..., Any]) -> Callable[..., DReturnType]: +def convertToReturnValue(func: Callable[P, T]) -> Callable[P, DReturnType[T]]: """Decorate a function to convert return values to `S_OK`/`S_ERROR` If `func` returns, wrap the return value in `S_OK`. @@ -207,18 +234,20 @@ def convertToReturnValue(func: Callable[..., Any]) -> Callable[..., DReturnType] """ @functools.wraps(func) - def wrapped(*args, **kwargs): + def wrapped(*args: P.args, **kwargs: P.kwargs) -> DReturnType[T]: try: - return S_OK(func(*args, **kwargs)) + value = func(*args, **kwargs) except SErrorException as e: return e.result except Exception as e: retval = S_ERROR(repr(e)) # Replace CallStack with the one from the exception - exc_type, exc_value, exc_tb = sys.exc_info() - retval["ExecInfo"] = exc_type, exc_value, exc_tb + retval["ExecInfo"] = sys.exc_info() + exc_type, exc_value, exc_tb = retval["ExecInfo"] retval["CallStack"] = traceback.format_tb(exc_tb) return retval + else: + return S_OK(value) # functools will copy the annotations. Since we change the return type # we have to update it From 0d61a4658ebf3de037173f1cb9fcafbdf8d78519 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Mon, 15 Aug 2022 10:09:35 +0200 Subject: [PATCH 5/7] fix (Core): Correct typing of S_ERROR()["ExecInfo"] --- src/DIRAC/Core/Utilities/ReturnValues.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/DIRAC/Core/Utilities/ReturnValues.py b/src/DIRAC/Core/Utilities/ReturnValues.py index 61f859ed16f..a5db7cb06f5 100755 --- a/src/DIRAC/Core/Utilities/ReturnValues.py +++ b/src/DIRAC/Core/Utilities/ReturnValues.py @@ -34,9 +34,7 @@ class DErrorReturnType(TypedDict): OK: Literal[False] Message: str Errno: int - ExecInfo: NotRequired[ - Union[tuple[None, None, None], tuple[Type[BaseException], BaseException, TracebackType]], - ] + ExecInfo: NotRequired[tuple[Type[BaseException], BaseException, TracebackType]] CallStack: NotRequired[list[str]] @@ -215,7 +213,7 @@ def returnValueOrRaise(result: DReturnType[T]) -> T: If no exception is known an :exc:`SErrorException` is raised. """ if not result["OK"]: - if "ExecInfo" in result and result["ExecInfo"][0]: + if "ExecInfo" in result: raise result["ExecInfo"][0] else: raise SErrorException(result) @@ -242,9 +240,10 @@ def wrapped(*args: P.args, **kwargs: P.kwargs) -> DReturnType[T]: except Exception as e: retval = S_ERROR(repr(e)) # Replace CallStack with the one from the exception - retval["ExecInfo"] = sys.exc_info() - exc_type, exc_value, exc_tb = retval["ExecInfo"] - retval["CallStack"] = traceback.format_tb(exc_tb) + exc_type, exc_value, exc_tb = sys.exc_info() + if exc_type and exc_value and exc_tb: + retval["ExecInfo"] = exc_type, exc_value, exc_tb + retval["CallStack"] = traceback.format_tb(exc_tb) return retval else: return S_OK(value) From 4c171ce09334bad51b127b348fb53d41848ef15a Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Mon, 15 Aug 2022 10:17:37 +0200 Subject: [PATCH 6/7] fix (Core): Use cast to correct typing of S_ERROR()["ExecInfo"] --- src/DIRAC/Core/Utilities/ReturnValues.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/DIRAC/Core/Utilities/ReturnValues.py b/src/DIRAC/Core/Utilities/ReturnValues.py index a5db7cb06f5..2249fbd69b2 100755 --- a/src/DIRAC/Core/Utilities/ReturnValues.py +++ b/src/DIRAC/Core/Utilities/ReturnValues.py @@ -240,10 +240,10 @@ def wrapped(*args: P.args, **kwargs: P.kwargs) -> DReturnType[T]: except Exception as e: retval = S_ERROR(repr(e)) # Replace CallStack with the one from the exception - exc_type, exc_value, exc_tb = sys.exc_info() - if exc_type and exc_value and exc_tb: - retval["ExecInfo"] = exc_type, exc_value, exc_tb - retval["CallStack"] = traceback.format_tb(exc_tb) + # Use cast as mypy doesn't understand that sys.exc_info can't return None in an exception block + retval["ExecInfo"] = cast(tuple[Type[BaseException], BaseException, TracebackType], sys.exc_info()) + exc_type, exc_value, exc_tb = retval["ExecInfo"] + retval["CallStack"] = traceback.format_tb(exc_tb) return retval else: return S_OK(value) From d1d07c42c52c0cd8483688614bb731b79e67b004 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Tue, 16 Aug 2022 18:15:34 +0200 Subject: [PATCH 7/7] fix: Depend on typing_extensions >=4.3.0 for generic TypedDict support --- environment.yml | 1 + setup.cfg | 1 + 2 files changed, 2 insertions(+) diff --git a/environment.yml b/environment.yml index edbad82dcdd..93e9c6cf822 100644 --- a/environment.yml +++ b/environment.yml @@ -50,6 +50,7 @@ dependencies: - diraccfg - ldap3 - importlib_resources + - typing_extensions >=4.3.0 # testing and development - pre-commit - coverage diff --git a/setup.cfg b/setup.cfg index 9d1e6524f29..4fbdb6e2665 100644 --- a/setup.cfg +++ b/setup.cfg @@ -50,6 +50,7 @@ install_requires = setuptools six sqlalchemy + typing_extensions >=4.3.0 Authlib >=1.0.0.a2 pyjwt dominate