Skip to content

Commit

Permalink
Issue #112 remove AggregatorConfig.aggregator_backends
Browse files Browse the repository at this point in the history
  • Loading branch information
soxofaan committed Mar 1, 2024
1 parent d74d57a commit 2e7a272
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 42 deletions.
7 changes: 1 addition & 6 deletions src/openeo_aggregator/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ class AggregatorConfig(dict):

config_source = dict_item()

# Dictionary mapping backend id to backend url
# TODO #112 deprecated, instead use by AggregatorBackendConfig.aggregator_backends
aggregator_backends = dict_item()

# TODO #112 deprecated, instead use AggregatorBackendConfig.partitioned_job_tracking
partitioned_job_tracking = dict_item(default=None)

Expand Down Expand Up @@ -129,8 +125,7 @@ class AggregatorBackendConfig(OpenEoBackendConfig):
packages=["openeo", "openeo_driver", "openeo_aggregator"],
)

# TODO #112 temporary default to allow migration, but make this field mandatory (and require non-empty)
aggregator_backends: Dict[str, str] = attrs.Factory(dict)
aggregator_backends: Dict[str, str] = attrs.field(validator=attrs.validators.min_len(1))

# See `ZooKeeperPartitionedJobDB.from_config` for supported fields.
partitioned_job_tracking: Optional[dict] = None
Expand Down
2 changes: 1 addition & 1 deletion src/openeo_aggregator/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def __init__(
def from_config(config: AggregatorConfig) -> 'MultiBackendConnection':
backend_config = get_backend_config()
return MultiBackendConnection(
backends=backend_config.aggregator_backends or config.aggregator_backends,
backends=backend_config.aggregator_backends,
configured_oidc_providers=backend_config.oidc_providers,
memoizer=memoizer_from_config(config, namespace="mbcon"),
connections_cache_ttl=backend_config.connections_cache_ttl,
Expand Down
2 changes: 1 addition & 1 deletion src/openeo_aggregator/metadata/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def main():
config: AggregatorConfig = get_config(args.environment)

# Determine backend ids/urls
aggregator_backends = get_backend_config().aggregator_backends or config.aggregator_backends
aggregator_backends = get_backend_config().aggregator_backends
backend_ids = args.backends or list(aggregator_backends.keys())
try:
backend_urls = [aggregator_backends[b] for b in backend_ids]
Expand Down
8 changes: 5 additions & 3 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,16 @@ def _get_config_content(config_var_name: str = "config"):
)


@pytest.mark.xfail(
reason="TODO #112: `aggregator_backends` should be mandatory, but to allow migration, it can currently be omitted."
)
def test_config_defaults():
with pytest.raises(TypeError, match="missing.*required.*aggregator_backends"):
_ = AggregatorBackendConfig()


def test_config_aggregator_backendsempty():
with pytest.raises(ValueError, match="Length of 'aggregator_backends' must be >= 1"):
_ = AggregatorBackendConfig(aggregator_backends={})


def test_config_aggregator_backends():
config = AggregatorBackendConfig(aggregator_backends={"b1": "https://b1.test"})
assert config.aggregator_backends == {"b1": "https://b1.test"}
Expand Down
31 changes: 0 additions & 31 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,37 +233,6 @@ def test_from_config(self, backend1, backend2):
backends = MultiBackendConnection.from_config(config)
assert set(c.id for c in backends.get_connections()) == {"b1", "b2"}

def test_from_config_legacy(self, requests_mock, mbldr):
"""Test `MultiBackendConnection.from_config` with legacy AggregatorConfig"""
# TODO #112 remove this test once AggregatorConfig is gone
requests_mock.get("https://b7.test/", json=mbldr.capabilities())
requests_mock.get("https://b7.test/credentials/oidc", json=mbldr.credentials_oidc())
requests_mock.get("https://b8.test/", json=mbldr.capabilities())
requests_mock.get("https://b8.test/credentials/oidc", json=mbldr.credentials_oidc())

config = AggregatorConfig(
aggregator_backends={"b7": "https://b7.test", "b8": "https://b8.test"},
)
with config_overrides(aggregator_backends={}):
backends = MultiBackendConnection.from_config(config)

assert set(c.id for c in backends.get_connections()) == {"b7", "b8"}

def test_from_config_new_and_legacy(self, requests_mock, mbldr):
"""Test `MultiBackendConnection.from_config` with legacy AggregatorConfig and new AggregatorBackendConfig"""
# TODO #112 remove this test once AggregatorConfig is gone
requests_mock.get("https://b7.test/", json=mbldr.capabilities())
requests_mock.get("https://b7.test/credentials/oidc", json=mbldr.credentials_oidc())
requests_mock.get("https://b8.test/", json=mbldr.capabilities())
requests_mock.get("https://b8.test/credentials/oidc", json=mbldr.credentials_oidc())

config = AggregatorConfig(
aggregator_backends={"b7": "https://b7.test"},
)
with config_overrides(aggregator_backends={"b8": "https://b8.test"}):
backends = MultiBackendConnection.from_config(config)

assert set(c.id for c in backends.get_connections()) == {"b8"}

@pytest.mark.parametrize(["bid1", "bid2"], [
("b1", "b1-dev"), ("b1", "b1.dev"), ("b1", "b1:dev"),
Expand Down

0 comments on commit 2e7a272

Please sign in to comment.