From 01760921f7c25af5f6ad1dd973b404047b411654 Mon Sep 17 00:00:00 2001 From: Reid Mello <30907815+rjmello@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:01:02 -0500 Subject: [PATCH] Re-enable support for extra args in EP config models A recent PR removed the `extra = "allow"` attribute from the `BaseConfigModel` Pydantic class. Many subclass models rely on this setting to permit required arguments without rasing a `ValidationError`. --- ...241205_100931_30907815+rjmello_v2_32_1.rst | 5 +++ .../endpoint/config/__init__.py | 1 + .../endpoint/config/model.py | 41 +++++++++---------- .../endpoint/config/utils.py | 6 ++- .../tests/unit/test_endpoint_config.py | 17 ++++++++ 5 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 changelog.d/20241205_100931_30907815+rjmello_v2_32_1.rst diff --git a/changelog.d/20241205_100931_30907815+rjmello_v2_32_1.rst b/changelog.d/20241205_100931_30907815+rjmello_v2_32_1.rst new file mode 100644 index 000000000..c2220a1e2 --- /dev/null +++ b/changelog.d/20241205_100931_30907815+rjmello_v2_32_1.rst @@ -0,0 +1,5 @@ +Bug Fixes +^^^^^^^^^ + +- Fixed an issue where valid endpoint configuration variables were ignored, + causing spurious validation errors. \ No newline at end of file diff --git a/compute_endpoint/globus_compute_endpoint/endpoint/config/__init__.py b/compute_endpoint/globus_compute_endpoint/endpoint/config/__init__.py index af2693c0b..4656f77b5 100644 --- a/compute_endpoint/globus_compute_endpoint/endpoint/config/__init__.py +++ b/compute_endpoint/globus_compute_endpoint/endpoint/config/__init__.py @@ -6,6 +6,7 @@ ) from .model import ( # noqa: F401 BaseConfigModel, + BaseEndpointConfigModel, ManagerEndpointConfigModel, UserEndpointConfigModel, ) diff --git a/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py b/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py index c2fe3f580..ef7ecfe20 100644 --- a/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py +++ b/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py @@ -43,16 +43,8 @@ def inner(cls, model: t.Optional[BaseModel]): class BaseConfigModel(BaseModel): - multi_user: t.Optional[bool] - display_name: t.Optional[str] - allowed_functions: t.Optional[t.List[uuid.UUID]] - authentication_policy: t.Optional[uuid.UUID] - subscription_id: t.Optional[uuid.UUID] - amqp_port: t.Optional[int] - heartbeat_period: t.Optional[int] - environment: t.Optional[str] - local_compute_services: t.Optional[bool] - debug: t.Optional[bool] + class Config: + extra = "allow" class AddressModel(BaseConfigModel): @@ -80,9 +72,6 @@ class ChannelModel(BaseConfigModel): class ProviderModel(BaseConfigModel): - class Config: - extra = "allow" - type: str channel: t.Optional[ChannelModel] launcher: t.Optional[LauncherModel] @@ -153,7 +142,23 @@ def _validate_provider_container_compatibility(cls, values: dict): return values -class UserEndpointConfigModel(BaseConfigModel): +class BaseEndpointConfigModel(BaseModel): + multi_user: t.Optional[bool] + display_name: t.Optional[str] + allowed_functions: t.Optional[t.List[uuid.UUID]] + authentication_policy: t.Optional[uuid.UUID] + subscription_id: t.Optional[uuid.UUID] + amqp_port: t.Optional[int] + heartbeat_period: t.Optional[int] + environment: t.Optional[str] + local_compute_services: t.Optional[bool] + debug: t.Optional[bool] + + class Config: + extra = "forbid" + + +class UserEndpointConfigModel(BaseEndpointConfigModel): engine: EngineModel heartbeat_threshold: t.Optional[int] idle_heartbeats_soft: t.Optional[int] @@ -167,9 +172,6 @@ class UserEndpointConfigModel(BaseConfigModel): _validate_engine = _validate_params("engine") - class Config: - extra = "forbid" - def dict(self, *args, **kwargs): # Slight modification is needed here since we still # store the engine/executor in a list named executors @@ -180,11 +182,8 @@ def dict(self, *args, **kwargs): return ret -class ManagerEndpointConfigModel(BaseConfigModel): +class ManagerEndpointConfigModel(BaseEndpointConfigModel): public: t.Optional[bool] identity_mapping_config_path: t.Optional[FilePath] force_mu_allow_same_user: t.Optional[bool] mu_child_ep_grace_period_s: t.Optional[float] - - class Config: - extra = "forbid" diff --git a/compute_endpoint/globus_compute_endpoint/endpoint/config/utils.py b/compute_endpoint/globus_compute_endpoint/endpoint/config/utils.py index bc9fe94e6..b3f75921a 100644 --- a/compute_endpoint/globus_compute_endpoint/endpoint/config/utils.py +++ b/compute_endpoint/globus_compute_endpoint/endpoint/config/utils.py @@ -90,10 +90,12 @@ def load_config_yaml(config_str: str) -> UserEndpointConfig | ManagerEndpointCon try: ConfigClass: type[UserEndpointConfig | ManagerEndpointConfig] if is_templatable: - from . import BaseConfigModel, ManagerEndpointConfigModel + from . import BaseEndpointConfigModel, ManagerEndpointConfigModel ConfigClass = ManagerEndpointConfig - config_schema: BaseConfigModel = ManagerEndpointConfigModel(**config_dict) + config_schema: BaseEndpointConfigModel = ManagerEndpointConfigModel( + **config_dict + ) else: from . import UserEndpointConfigModel diff --git a/compute_endpoint/tests/unit/test_endpoint_config.py b/compute_endpoint/tests/unit/test_endpoint_config.py index 661a6b791..61c6d73e5 100644 --- a/compute_endpoint/tests/unit/test_endpoint_config.py +++ b/compute_endpoint/tests/unit/test_endpoint_config.py @@ -227,3 +227,20 @@ def test_managerconfig_repr_nondefault_kwargs( repr_c = repr(ManagerEndpointConfig(**{kw: val})) assert f"{kw}={repr(val)}" in repr_c + + +def test_engine_model_objects_allow_extra(): + config_dict = { + "engine": { + "type": "GlobusComputeEngine", + "address": { + "type": "address_by_interface", + "ifname": "lo", # Not specified in model + }, + "provider": { + "type": "LocalProvider", + "max_blocks": 2, # Not specified in model + }, + } + } + UserEndpointConfigModel(**config_dict)