From ee0d714ccefe7465beb567c04938ed0e27f5b9a8 Mon Sep 17 00:00:00 2001 From: Dominik Gresch Date: Thu, 3 Oct 2024 09:08:24 +0200 Subject: [PATCH] Add 'supported_since' keywords to the gRPC property helpers Allow marking gRPC properties as supported since a specific server version. The `grpc_data_property_read_only` is given a `supported_since` keyword, and `grpc_data_property` is given two separate keywords `readable_since` and `writable_since`. Other changes: - Change the `xfail_before` test fixture to `raises_before_version`, which explicitly checks that a `RuntimeError` is raised when run on an older server version. - Move the `supported_since` implementation to a separate file. - In the CI definition, reuse the `DOCKER_IMAGE_NAME` variable in more places. --- .github/workflows/ci_cd.yml | 4 +- .../_grpc_helpers/property_helper.py | 98 ++++++++++++++----- .../_tree_objects/_grpc_helpers/protocols.py | 4 + .../_grpc_helpers/supported_since.py | 84 ++++++++++++++++ src/ansys/acp/core/_tree_objects/base.py | 39 ++------ src/ansys/acp/core/_tree_objects/model.py | 3 +- tests/conftest.py | 8 +- tests/unittests/test_model.py | 11 ++- 8 files changed, 186 insertions(+), 65 deletions(-) create mode 100644 src/ansys/acp/core/_tree_objects/_grpc_helpers/supported_since.py diff --git a/.github/workflows/ci_cd.yml b/.github/workflows/ci_cd.yml index 36d521c5ac..96efef7173 100644 --- a/.github/workflows/ci_cd.yml +++ b/.github/workflows/ci_cd.yml @@ -170,7 +170,7 @@ jobs: poetry run pytest -v --license-server=1055@$LICENSE_SERVER --no-server-log-files --docker-image=$IMAGE_NAME --cov=ansys.acp.core --cov-report=term --cov-report=xml --cov-report=html env: LICENSE_SERVER: ${{ secrets.LICENSE_SERVER }} - IMAGE_NAME: "ghcr.io/ansys/acp${{ github.event.inputs.docker_image_suffix || ':latest' }}" + IMAGE_NAME: ${{ env.DOCKER_IMAGE_NAME }} - name: "Upload coverage to Codecov" uses: codecov/codecov-action@v4 @@ -279,7 +279,7 @@ jobs: run: > poetry run ansys-launcher configure ACP docker_compose - --image_name_pyacp=ghcr.io/ansys/acp${{ github.event.inputs.docker_image_suffix || ':latest' }} + --image_name_pyacp=${{ env.DOCKER_IMAGE_NAME }} --image_name_filetransfer=ghcr.io/ansys/tools-filetransfer:latest --license_server=1055@$LICENSE_SERVER --keep_volume=False diff --git a/src/ansys/acp/core/_tree_objects/_grpc_helpers/property_helper.py b/src/ansys/acp/core/_tree_objects/_grpc_helpers/property_helper.py index 50bac3c7e4..9f3ae90735 100644 --- a/src/ansys/acp/core/_tree_objects/_grpc_helpers/property_helper.py +++ b/src/ansys/acp/core/_tree_objects/_grpc_helpers/property_helper.py @@ -38,6 +38,7 @@ from ..._utils.property_protocols import ReadOnlyProperty, ReadWriteProperty from .polymorphic_from_pb import CreatableFromResourcePath, tree_object_from_resource_path from .protocols import Editable, GrpcObjectBase, ObjectInfo, Readable +from .supported_since import supported_since as supported_since_decorator # Note: The typing of the protobuf objects is fairly loose, maybe it could # be improved. The main challenge is that we do not encode the structure of @@ -110,7 +111,10 @@ def inner(self: Readable) -> CreatableFromResourcePath | None: def grpc_data_getter( - name: str, from_protobuf: _FROM_PROTOBUF_T[_GET_T], check_optional: bool = False + name: str, + from_protobuf: _FROM_PROTOBUF_T[_GET_T], + check_optional: bool = False, + supported_since: str | None = None, ) -> Callable[[Readable], _GET_T]: """Create a getter method which obtains the server object via the gRPC Get endpoint. @@ -125,6 +129,14 @@ def grpc_data_getter( will be used. """ + @supported_since_decorator( + supported_since, + # The default error message uses 'inner' as the method name, which is confusing + err_msg_tpl=( + f"The property '{name.split('.')[-1]}' is only readable since version {{required_version}} " + f"of the ACP gRPC server. The current server version is {{server_version}}." + ), + ) def inner(self: Readable) -> Any: self._get_if_stored() pb_attribute = _get_data_attribute(self._pb_object, name, check_optional=check_optional) @@ -149,26 +161,6 @@ def inner(self: Editable, value: Readable | None) -> None: return inner -def grpc_data_setter( - name: str, to_protobuf: _TO_PROTOBUF_T[_SET_T] -) -> Callable[[Editable, _SET_T], None]: - """Create a setter method which updates the server object via the gRPC Put endpoint.""" - - def inner(self: Editable, value: _SET_T) -> None: - self._get_if_stored() - current_value = _get_data_attribute(self._pb_object, name) - value_pb = to_protobuf(value) - try: - needs_updating = current_value != value_pb - except TypeError: - needs_updating = True - if needs_updating: - _set_data_attribute(self._pb_object, name, value_pb) - self._put_if_stored() - - return inner - - def _get_data_attribute(pb_obj: Message, name: str, check_optional: bool = False) -> _PROTOBUF_T: name_parts = name.split(".") if check_optional: @@ -197,6 +189,37 @@ def _set_data_attribute(pb_obj: ObjectInfo, name: str, value: _PROTOBUF_T) -> No target_object.add().CopyFrom(item) +def grpc_data_setter( + name: str, + to_protobuf: _TO_PROTOBUF_T[_SET_T], + setter_func: Callable[[ObjectInfo, str, _PROTOBUF_T], None] = _set_data_attribute, + supported_since: str | None = None, +) -> Callable[[Editable, _SET_T], None]: + """Create a setter method which updates the server object via the gRPC Put endpoint.""" + + @supported_since_decorator( + supported_since, + # The default error message uses 'inner' as the method name, which is confusing + err_msg_tpl=( + f"The property '{name.split('.')[-1]}' is only editable since version {{required_version}} " + f"of the ACP gRPC server. The current server version is {{server_version}}." + ), + ) + def inner(self: Editable, value: _SET_T) -> None: + self._get_if_stored() + current_value = _get_data_attribute(self._pb_object, name) + value_pb = to_protobuf(value) + try: + needs_updating = current_value != value_pb + except TypeError: + needs_updating = True + if needs_updating: + setter_func(self._pb_object, name, value_pb) + self._put_if_stored() + + return inner + + AnyT = TypeVar("AnyT") @@ -212,6 +235,9 @@ def grpc_data_property( from_protobuf: _FROM_PROTOBUF_T[_GET_T] = lambda x: x, check_optional: bool = False, doc: str | None = None, + setter_func: Callable[[ObjectInfo, str, _PROTOBUF_T], None] = _set_data_attribute, + readable_since: str | None = None, + writable_since: str | None = None, ) -> ReadWriteProperty[_GET_T, _SET_T]: """Define a property which is synchronized with the backend via gRPC. @@ -234,6 +260,10 @@ def grpc_data_property( will be used. doc : Docstring for the property. + readable_since : + Version since which the property is supported for reading. + writable_since : + Version since which the property is supported for setting. """ # Note jvonrick August 2023: We don't ensure with typechecks that the property returned here is # compatible with the class on which this property is created. For example: @@ -244,8 +274,20 @@ def grpc_data_property( # https://github.com/python/typing/issues/985 return _wrap_doc( _exposed_grpc_property( - grpc_data_getter(name, from_protobuf=from_protobuf, check_optional=check_optional) - ).setter(grpc_data_setter(name, to_protobuf=to_protobuf)), + grpc_data_getter( + name, + from_protobuf=from_protobuf, + check_optional=check_optional, + supported_since=readable_since, + ) + ).setter( + grpc_data_setter( + name, + to_protobuf=to_protobuf, + setter_func=setter_func, + supported_since=writable_since, + ) + ), doc=doc, ) @@ -255,6 +297,7 @@ def grpc_data_property_read_only( from_protobuf: _FROM_PROTOBUF_T[_GET_T] = lambda x: x, check_optional: bool = False, doc: str | None = None, + supported_since: str | None = None, ) -> ReadOnlyProperty[_GET_T]: """Define a read-only property which is synchronized with the backend via gRPC. @@ -275,10 +318,17 @@ def grpc_data_property_read_only( will be used. doc : Docstring for the property. + supported_since : + Version since which the property is supported. """ return _wrap_doc( _exposed_grpc_property( - grpc_data_getter(name, from_protobuf=from_protobuf, check_optional=check_optional) + grpc_data_getter( + name, + from_protobuf=from_protobuf, + check_optional=check_optional, + supported_since=supported_since, + ) ), doc=doc, ) diff --git a/src/ansys/acp/core/_tree_objects/_grpc_helpers/protocols.py b/src/ansys/acp/core/_tree_objects/_grpc_helpers/protocols.py index fdc2410dda..35bfdea392 100644 --- a/src/ansys/acp/core/_tree_objects/_grpc_helpers/protocols.py +++ b/src/ansys/acp/core/_tree_objects/_grpc_helpers/protocols.py @@ -29,6 +29,7 @@ from google.protobuf.message import Message import grpc +from packaging.version import Version from ansys.api.acp.v0.base_pb2 import ( BasicInfo, @@ -188,6 +189,9 @@ def _resource_path(self) -> ResourcePath: ... _pb_object: Any + @property + def _server_version(self) -> Version | None: ... + class Editable(Readable, Protocol): """Interface definition for editable objects.""" diff --git a/src/ansys/acp/core/_tree_objects/_grpc_helpers/supported_since.py b/src/ansys/acp/core/_tree_objects/_grpc_helpers/supported_since.py new file mode 100644 index 0000000000..e27b63ff5e --- /dev/null +++ b/src/ansys/acp/core/_tree_objects/_grpc_helpers/supported_since.py @@ -0,0 +1,84 @@ +# Copyright (C) 2022 - 2024 ANSYS, Inc. and/or its affiliates. +# SPDX-License-Identifier: MIT +# +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +from collections.abc import Callable +from functools import wraps +from typing import Concatenate, TypeAlias, TypeVar + +from packaging.version import parse as parse_version +from typing_extensions import ParamSpec + +from .protocols import Readable + +T = TypeVar("T", bound=Readable) +P = ParamSpec("P") +R = TypeVar("R") +_WRAPPED_T: TypeAlias = Callable[Concatenate[T, P], R] + + +def supported_since( + version: str | None, err_msg_tpl: str | None = None +) -> Callable[[_WRAPPED_T[T, P, R]], _WRAPPED_T[T, P, R]]: + """Mark a TreeObjectBase method as supported since a specific server version. + + Raises an exception if the current server version does not match the required version. + If either the given `version` or the server version is `None`, the decorator does nothing. + + Parameters + ---------- + version : Optional[str] + The server version since which the method is supported. If ``None``, the + decorator does nothing. + err_msg_tpl : Optional[str] + A custom error message template. If ``None``, a default error message is used. + """ + if version is None: + # return a trivial decorator if no version is specified + def trivial_decorator(func: _WRAPPED_T[T, P, R]) -> _WRAPPED_T[T, P, R]: + return func + + return trivial_decorator + + required_version = parse_version(version) + + def decorator(func: _WRAPPED_T[T, P, R]) -> _WRAPPED_T[T, P, R]: + @wraps(func) + def inner(self: T, /, *args: P.args, **kwargs: P.kwargs) -> R: + server_version = self._server_version + # If the object is not stored, we cannot check the server version. + if server_version is not None: + if server_version < required_version: + if err_msg_tpl is None: + err_msg = ( + f"The method '{func.__name__}' is only supported since version {version} " + f"of the ACP gRPC server. The current server version is {server_version}." + ) + else: + err_msg = err_msg_tpl.format( + required_version=required_version, server_version=server_version + ) + raise RuntimeError(err_msg) + return func(self, *args, **kwargs) + + return inner + + return decorator diff --git a/src/ansys/acp/core/_tree_objects/base.py b/src/ansys/acp/core/_tree_objects/base.py index 758f993719..8e0a11133f 100644 --- a/src/ansys/acp/core/_tree_objects/base.py +++ b/src/ansys/acp/core/_tree_objects/base.py @@ -26,14 +26,13 @@ from abc import abstractmethod from collections.abc import Callable, Iterable from dataclasses import dataclass -from functools import wraps import typing -from typing import Any, Concatenate, Generic, TypeAlias, TypeVar, cast +from typing import Any, Generic, TypeVar, cast from grpc import Channel from packaging.version import Version from packaging.version import parse as parse_version -from typing_extensions import ParamSpec, Self +from typing_extensions import Self from ansys.api.acp.v0.base_pb2 import CollectionPath, DeleteRequest, GetRequest, ResourcePath @@ -147,6 +146,12 @@ def _server_wrapper(self) -> ServerWrapper: assert self._server_wrapper_store is not None return self._server_wrapper_store + @property + def _server_version(self) -> Version | None: + if not self._is_stored: + return None + return self._server_wrapper.version + @property def _is_stored(self) -> bool: return self._server_wrapper_store is not None @@ -478,34 +483,6 @@ def _put_if_stored(self) -> None: self._put() -T = TypeVar("T", bound=TreeObjectBase) -P = ParamSpec("P") -R = TypeVar("R") -_WRAPPED_T: TypeAlias = Callable[Concatenate[T, P], R] - - -def supported_since(version: str) -> Callable[[_WRAPPED_T[T, P, R]], _WRAPPED_T[T, P, R]]: - """Mark a TreeObjectBase method as supported since a specific server version. - - Raises an exception if the current server version does not match the required version. - """ - required_version = parse_version(version) - - def decorator(func: _WRAPPED_T[T, P, R]) -> _WRAPPED_T[T, P, R]: - @wraps(func) - def inner(self: T, /, *args: P.args, **kwargs: P.kwargs) -> R: - if self._server_wrapper.version < required_version: - raise RuntimeError( - f"The method '{func.__name__}' is only supported since version {version} of the ACP " - f"gRPC server. The current server version is {self._server_wrapper.version}." - ) - return func(self, *args, **kwargs) - - return inner - - return decorator - - if typing.TYPE_CHECKING: # pragma: no cover # Ensure that the ReadOnlyTreeObject satisfies the Gettable interface _x: Readable = typing.cast(ReadOnlyTreeObject, None) diff --git a/src/ansys/acp/core/_tree_objects/model.py b/src/ansys/acp/core/_tree_objects/model.py index b4fec9c781..3efca801ba 100644 --- a/src/ansys/acp/core/_tree_objects/model.py +++ b/src/ansys/acp/core/_tree_objects/model.py @@ -79,6 +79,7 @@ grpc_data_property_read_only, mark_grpc_properties, ) +from ._grpc_helpers.supported_since import supported_since from ._mesh_data import ( ElementalData, NodalData, @@ -87,7 +88,7 @@ elemental_data_property, nodal_data_property, ) -from .base import ServerWrapper, TreeObject, supported_since +from .base import ServerWrapper, TreeObject from .boolean_selection_rule import BooleanSelectionRule from .cad_geometry import CADGeometry from .cutoff_selection_rule import CutoffSelectionRule diff --git a/tests/conftest.py b/tests/conftest.py index e09540c005..d6b22b2828 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -264,11 +264,15 @@ def inner(model, relative_file_path="square_and_solid.stp"): @pytest.fixture -def xfail_before(acp_instance): +def raises_before_version(acp_instance): """Mark a test as expected to fail before a certain server version.""" + @contextmanager def inner(version: str): if parse_version(acp_instance.server_version) < parse_version(version): - pytest.xfail(f"Expected to fail until server version {version!r}") + with pytest.raises(RuntimeError): + yield + else: + yield return inner diff --git a/tests/unittests/test_model.py b/tests/unittests/test_model.py index eb86bff3d2..f0a4483005 100644 --- a/tests/unittests/test_model.py +++ b/tests/unittests/test_model.py @@ -251,12 +251,11 @@ def test_regression_454(minimal_complete_model): assert not hasattr(minimal_complete_model, "store") -def test_modeling_ply_export(acp_instance, minimal_complete_model, xfail_before): +def test_modeling_ply_export(acp_instance, minimal_complete_model, raises_before_version): """ Test that the 'export_modeling_ply_geometries' method produces a file. The contents of the file are not checked. """ - xfail_before("25.1") out_filename = "modeling_ply_export.step" with tempfile.TemporaryDirectory() as tmp_dir: @@ -265,9 +264,11 @@ def test_modeling_ply_export(acp_instance, minimal_complete_model, xfail_before) out_file_path = pathlib.Path(out_filename) else: out_file_path = local_file_path - minimal_complete_model.export_modeling_ply_geometries(out_file_path) - acp_instance.download_file(out_file_path, local_file_path) - assert local_file_path.exists() + + with raises_before_version("25.1"): + minimal_complete_model.export_modeling_ply_geometries(out_file_path) + acp_instance.download_file(out_file_path, local_file_path) + assert local_file_path.exists() def test_parent_access_raises(minimal_complete_model):