Skip to content

Change type of TargetComponents #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Change type of TargetComponents #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d9fc31
Make `TargetComponents` public
ela-kotulska-frequenz May 26, 2025
22be2c2
Change type of TargetComponents to return ComponentId
ela-kotulska-frequenz May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,11 @@

## Summary

<!-- Here goes a general summary of what this release is about -->

## Upgrading

<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->

## New Features

* `dispatch-cli` supports now the parameter `--type` and `--running` to filter the list of running services by type and status, respectively.
* Every call now has a default timeout of 60 seconds, streams terminate after five minutes. This can be influenced by the two new parameters for`DispatchApiClient.__init__()`:
* `default_timeout: timedelta` (default: 60 seconds)
* `stream_timeout: timedelta` (default: 5 minutes)

## Bug Fixes

<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
* `frequenz.client.dispatch.TargetComponents` is now public, and its type has changed from `list[int] | list[ComponentCategory]` to `list[ComponentId] | list[ComponentCategory]`. This change introduces a new dependency on `frequenz-client-microgrid` (>= v0.7.0, < 0.8.0) for the `frequenz.client.microgrid.ComponentId` type.
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ dependencies = [
"frequenz-api-dispatch == 1.0.0-rc1",
"frequenz-client-base >= 0.8.0, < 0.10.0",
"frequenz-client-common >= 0.1.0, < 0.4.0",
"frequenz-client-microgrid >= 0.7.0, < 0.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not do this yet. If needed we should move it to frequenz-client-common. I will do a PR soon.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I need this now to move actor to latest SDK :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if it is temporary is OK, but please create an issue so we don't forget to remove the dependency in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a second thought, it is not necessary to start using ComponentId, you can always convert between it and int freely (cid = ComponentId(1) / id = int(cid)).

Copy link
Author

@ela-kotulska-frequenz ela-kotulska-frequenz May 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TargetComponents is part of DispatchRequest.
Actor receives DispatchRequest and needs to create BatteryPool from the target.
BatteryPool expects set[ComponentId] :)

I can to change every actor to change int to ComponentId.
But after we add this, I would have to revert this change in every actor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmm, damn. I guess then I'm back to the "OK as a temporary thing" comment 😬

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please create an issue so we don't forget to remove the dependency in the future.

#172

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use typing.SupportsInt and support the unknown to int castable ComponentsId that way

Copy link
Author

@ela-kotulska-frequenz ela-kotulska-frequenz May 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would it fix it? The problem with Actor will still be there. They except ComponentId, get int.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marenz had awesome idea to make this change temporarily somewhere else :)
frequenz-floss/frequenz-dispatch-python#156

"grpcio >= 1.66.1, < 2",
"python-dateutil >= 2.8.2, < 3.0",
]
Expand Down
3 changes: 2 additions & 1 deletion src/frequenz/client/dispatch/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"""Dispatch API client for Python."""

from ._client import DispatchApiClient
from .types import TargetComponents

__all__ = ["DispatchApiClient"]
__all__ = ["DispatchApiClient", "TargetComponents"]
7 changes: 5 additions & 2 deletions src/frequenz/client/dispatch/_cli_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from tzlocal import get_localzone

from frequenz.client.common.microgrid.components import ComponentCategory
from frequenz.client.microgrid import ComponentId

# Disable a false positive from pylint
# pylint: disable=inconsistent-return-statements
Expand Down Expand Up @@ -140,7 +141,7 @@ class TargetComponentParamType(click.ParamType):

def convert(
self, value: Any, param: click.Parameter | None, ctx: click.Context | None
) -> list[ComponentCategory] | list[int]:
) -> list[ComponentCategory] | list[ComponentId]:
"""Convert the input value into a list of ComponentCategory or IDs.

Args:
Expand All @@ -152,6 +153,8 @@ def convert(
A list of component ids or component categories.
"""
if isinstance(value, list): # Already a list
if all(isinstance(item, int) for item in value):
return list(map(ComponentId, value))
return value

values = value.split(",")
Expand All @@ -162,7 +165,7 @@ def convert(
error: Exception | None = None
# Attempt to parse component ids
try:
return [int(id) for id in values]
return [ComponentId(int(id)) for id in values]
except ValueError as e:
error = e

Expand Down
3 changes: 2 additions & 1 deletion src/frequenz/client/dispatch/test/generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from datetime import datetime, timedelta, timezone

from frequenz.client.common.microgrid.components import ComponentCategory
from frequenz.client.microgrid import ComponentId

from .._internal_types import rounded_start_time
from ..recurrence import EndCriteria, Frequency, RecurrenceRule, Weekday
Expand Down Expand Up @@ -99,7 +100,7 @@ def generate_dispatch(self) -> Dispatch:
for _ in range(self._rng.randint(1, 10))
],
[
self._rng.randint(1, 100)
ComponentId(self._rng.randint(1, 100))
for _ in range(self._rng.randint(1, 10))
],
]
Expand Down
17 changes: 12 additions & 5 deletions src/frequenz/client/dispatch/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@

# pylint: enable=no-name-in-module
from frequenz.client.common.microgrid.components import ComponentCategory
from frequenz.client.microgrid import ComponentId

from .recurrence import Frequency, RecurrenceRule, Weekday

TargetComponents = list[int] | list[ComponentCategory]
TargetComponents = list[ComponentId] | list[ComponentCategory]
"""One or more target components specifying which components a dispatch targets.

It can be a list of component IDs or a list of categories.
It can be a list of `ComponentId` instances (representing component IDs)
or a list of `ComponentCategory` instances (representing categories).
"""


Expand All @@ -50,7 +52,9 @@ def _target_components_from_protobuf(
"""
match pb_target.WhichOneof("components"):
case "component_ids":
id_list: list[int] = list(pb_target.component_ids.ids)
id_list: list[ComponentId] = list(
map(ComponentId, pb_target.component_ids.ids)
)
return id_list
case "component_categories":
category_list: list[ComponentCategory] = list(
Expand Down Expand Up @@ -80,8 +84,11 @@ def _target_components_to_protobuf(
"""
pb_target = PBTargetComponents()
match target:
case list(component_ids) if all(isinstance(id, int) for id in component_ids):
pb_target.component_ids.ids.extend(cast(list[int], component_ids))
case list(component_ids) if all(
isinstance(id, ComponentId) for id in component_ids
):
# pylint: disable-next=unnecessary-lambda
pb_target.component_ids.ids.extend(map(lambda x: int(x), component_ids))
case list(categories) if all(
isinstance(cat, ComponentCategory) for cat in categories
):
Expand Down
27 changes: 14 additions & 13 deletions tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
)
from frequenz.client.dispatch.test.client import ALL_KEY, FakeClient
from frequenz.client.dispatch.types import Dispatch
from frequenz.client.microgrid import ComponentId

TEST_NOW = datetime(2023, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
"""Arbitrary time used as NOW for testing."""
Expand Down Expand Up @@ -68,7 +69,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand All @@ -91,7 +92,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand All @@ -113,7 +114,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand All @@ -128,7 +129,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand Down Expand Up @@ -156,7 +157,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand All @@ -169,7 +170,7 @@ def mock_client(fake_client: FakeClient) -> Generator[None, None, None]:
type="filtered",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=1800),
target=[3],
target=list(map(ComponentId, [3])),
active=True,
dry_run=False,
payload={},
Expand Down Expand Up @@ -247,7 +248,7 @@ async def test_list_command(
"test",
timedelta(hours=2),
timedelta(seconds=3600),
[1, 2, 3],
list(map(ComponentId, [1, 2, 3])),
{"dry_run": True},
RecurrenceRule(),
0,
Expand Down Expand Up @@ -376,7 +377,7 @@ async def test_create_command(
expected_type: str,
expected_start_time_delta: timedelta | Literal["NOW"],
expected_duration: timedelta,
expected_target: list[int] | list[ComponentCategory],
expected_target: list[ComponentId] | list[ComponentCategory],
expected_options: dict[str, Any],
expected_reccurence: RecurrenceRule | None,
expected_return_code: int,
Expand Down Expand Up @@ -528,7 +529,7 @@ async def test_create_command(
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[500, 501],
target=list(map(ComponentId, [500, 501])),
active=True,
dry_run=False,
payload={},
Expand Down Expand Up @@ -558,7 +559,7 @@ async def test_create_command(
'{"key": "value"}',
],
{
"target": [400, 401],
"target": list(map(ComponentId, [400, 401])),
"recurrence": RecurrenceRule(
frequency=Frequency.DAILY,
interval=5,
Expand All @@ -574,7 +575,7 @@ async def test_create_command(
"payload": {"key": "value"},
},
0,
""" target=[400, 401],
""" target=[ComponentId(400), ComponentId(401)],
active=True,
dry_run=False,
payload={'key': 'value'},
Expand Down Expand Up @@ -633,7 +634,7 @@ async def test_update_command(
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand Down Expand Up @@ -680,7 +681,7 @@ async def test_get_command(
type="test",
start_time=datetime(2023, 1, 1, 0, 0, 0),
duration=timedelta(seconds=3600),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={},
Expand Down
11 changes: 6 additions & 5 deletions tests/test_proto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
_target_components_from_protobuf,
_target_components_to_protobuf,
)
from frequenz.client.microgrid import ComponentId


def test_target_components() -> None:
"""Test the target components."""
for components in (
[1, 2, 3],
[10, 20, 30],
list(map(ComponentId, [1, 2, 3])),
list(map(ComponentId, [10, 20, 30])),
[ComponentCategory.BATTERY],
[ComponentCategory.GRID],
[ComponentCategory.METER],
Expand Down Expand Up @@ -99,7 +100,7 @@ def test_dispatch() -> None:
start_time=datetime(2024, 10, 10, tzinfo=timezone.utc),
end_time=datetime(2024, 10, 20, tzinfo=timezone.utc),
duration=timedelta(days=10),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={"key": "value"},
Expand Down Expand Up @@ -163,7 +164,7 @@ def test_dispatch_create_request_with_no_recurrence() -> None:
type="test",
start_time=datetime(2024, 10, 10, tzinfo=timezone.utc),
duration=timedelta(days=10),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={"key": "value"},
Expand All @@ -180,7 +181,7 @@ def test_dispatch_create_start_immediately() -> None:
type="test",
start_time="NOW",
duration=timedelta(days=10),
target=[1, 2, 3],
target=list(map(ComponentId, [1, 2, 3])),
active=True,
dry_run=False,
payload={"key": "value"},
Expand Down
Loading