Skip to content

Commit

Permalink
Raise error for provider w/ internal container mgmt
Browse files Browse the repository at this point in the history
The endpoint CLI will now raise an error if the endpoint configuration
includes both the `container_uri` field and a provider that manages
containers internally. This prevents conflicts in container management.
  • Loading branch information
rjmello committed Aug 8, 2024
1 parent 016625f commit efd891b
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bug Fixes
^^^^^^^^^

- The endpoint CLI will now raise an error if the endpoint configuration includes
both the ``container_uri`` field and a provider that manages containers internally
(``AWSProvider``, ``GoogleCloudProvider``, or ``KubernetesProvider``). This prevents
conflicts in container management.
18 changes: 18 additions & 0 deletions compute_endpoint/globus_compute_endpoint/endpoint/config/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,24 @@ def _validate_engine_strategy(cls, values: dict):
)
return values

@root_validator(pre=True)
@classmethod
def _validate_provider_container_compatibility(cls, values: dict):
provider_type = values.get("provider", {}).get("type")
if provider_type in (
"AWSProvider",
"GoogleCloudProvider",
"KubernetesProvider",
):
if values.get("container_uri"):
raise ValueError(
f"The 'container_uri' field is not compatible with {provider_type}"
" because this provider manages containers internally. For more"
f" information on how to configure {provider_type}, please refer to"
f" Parsl documentation: https://parsl.readthedocs.io/en/stable/stubs/parsl.providers.{provider_type}.html" # noqa"
)
return values


class ConfigModel(BaseConfigModel):
engine: t.Optional[EngineModel]
Expand Down
23 changes: 23 additions & 0 deletions compute_endpoint/tests/unit/test_endpoint_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,26 @@ def test_conditional_engine_strategy(
with pytest.raises(ValidationError) as pyt_e:
ConfigModel(**config_dict)
assert "string is incompatible" in str(pyt_e.value)


@pytest.mark.parametrize(
"provider_type, compatible",
(
("LocalProvider", True),
("AWSProvider", False),
("GoogleCloudProvider", False),
("KubernetesProvider", False),
),
)
def test_provider_container_compatibility(
config_dict: dict, provider_type: str, compatible: bool
):
config_dict["engine"]["container_uri"] = "docker://ubuntu"
config_dict["engine"]["provider"] = {"type": provider_type}

if compatible:
ConfigModel(**config_dict)
else:
with pytest.raises(ValueError) as pyt_e:
ConfigModel(**config_dict)
assert f"not compatible with {provider_type}" in str(pyt_e.value)

0 comments on commit efd891b

Please sign in to comment.