Initial client #7

flora-hofmann-frequenz · 2025-02-25T14:10:01Z

First draft of initial client, tests still need to be added.

cwasicki · 2025-05-02T17:20:35Z

src/frequenz/client/assets/_client.py

+    manufacturer: str
+    model_name: str
+    status: str
+    start_timestamp: Optional[datetime]


{start,end}_time

src/frequenz/client/assets/_client.py

cwasicki · 2025-05-02T17:21:14Z

src/frequenz/client/assets/_client.py

+    id: int
+    enterprise_id: int
+    name: str
+    delivery_area_code: Optional[str]


Optional = str | None

cwasicki · 2025-05-02T17:21:49Z

src/frequenz/client/assets/_client.py

+            longitude=location.longitude if location else None,
+            country_code=location.country_code if location else None,
+            status=pb.status,
+            created_at=pb.create_timestamp.ToDatetime().replace(tzinfo=timezone.utc),


create_time

See https://google.aip.dev/142

src/frequenz/client/assets/_client.py

cwasicki · 2025-05-02T17:35:47Z

src/frequenz/client/assets/_client.py

+    microgrid_id: int
+    name: str
+    category: str
+    category_metadata: dict[str, Any]


This is a bit tricky, I think the type implementation for the metadata variant should also be implemented in common client.

Will be addressed after common-client wrappers have been reviewed

cwasicki · 2025-05-02T17:36:39Z

src/frequenz/client/assets/_client.py

+    category_metadata: dict[str, Any]
+    manufacturer: str
+    model_name: str
+    status: str


Same here, we need to use a python equivalent of that type (I think it exists also already in common).

Will be addressed after common-client wrappers have been reviewed

cwasicki · 2025-05-02T17:40:06Z

src/frequenz/client/assets/_client.py

+    status: str
+    start_timestamp: Optional[datetime]
+    end_timestamp: Optional[datetime]
+    metric_config_bounds: list[dict]


The metric could be the key and the bounds be in a dedicated dataclass. Maybe also worth in common?

Will be addressed after common-client wrappers have been reviewed

cwasicki · 2025-05-02T17:47:55Z

src/frequenz/client/assets/_client.py

+
+# pylint: disable=too-many-instance-attributes
+@dataclass(frozen=True)
+class Microgrid:


I am not sure if all these dataclasses here should go into the common client repo

Should we discuss this with someone else from the api-team?

Posted it in the assets-channel to get some opinions on it.

Will be addressed after common-client wrappers have been reviewed

cwasicki · 2025-05-02T17:52:14Z

src/frequenz/client/assets/_client.py

+        self,
+        microgrid_id: int,
+        source_component_ids: Optional[list[int]] = None,
+        destination_component_ids: Optional[list[int]] = None,


I think it would be useful to be able to optionally pass a timestamp and the connections are filtered such that only those relevant to that point in time are returned.

Still missing in service

It could be implemented in the client already since we have this information. However, it's not so easy since it would ideally support a time range, which could result in multiple graphs. Maybe that's something for the assets API tooling.

pyproject.toml

Signed-off-by: Flora <[email protected]>

flora-hofmann-frequenz · 2025-07-15T13:49:40Z

@cwasicki & @cyiallou: I would like to get this one merged if possible (it is working, but has been open very long due to dependencies on other repos). We can then improve on the open topics (such taking wrappers out and using the ones from client-common once available , generally updating the service / assets-api to common-api v0.8.0 etc) in follow up PRs. Costas, Richard & I will meet tomorrow to discuss open topics and next steps forward. Let me know if anything speaks against this plan of action.

cyiallou

Thanks for putting this together. I’m still getting up to speed with all the integration points across the various repos, so I’m not in a position to fully evaluate all the implementation details just yet. From what I can see, the client seems functional, but it looks like tests are still missing, and as you noted, some refactoring would be needed. I imagine those improvements could be done in follow-up PRs.

I will hold off on giving a formal approval for now and would appreciate if someone with more context around the broader system could take a closer look.

cwasicki · 2025-07-28T11:36:04Z

examples/client.py

@@ -0,0 +1,86 @@
+# License: MIT


This would be better implemented as a CLI tool similar to the reporting client.

Actually more like the CLI tool in trading, which provides an example for different endpoints.

cwasicki · 2025-07-28T11:38:00Z

examples/client.py

+        source_component_ids: List of source component IDs to filter.
+        destination_component_ids: List of destination component IDs to filter.
+    """
+    server_url = "localhost:50052"


Should be a command line argument.

cwasicki · 2025-07-28T11:41:56Z

src/frequenz/client/assets/_client.py

+
+    source_component_id: int
+    destination_component_id: int
+    start_time: Optional[datetime]


datetime | None

You can replace all Optionals in this module following this pattern

cwasicki · 2025-07-28T11:52:14Z

src/frequenz/client/assets/_client.py

+        self,
+        microgrid_id: int,
+        source_component_ids: Optional[list[int]] = None,
+        destination_component_ids: Optional[list[int]] = None,


It could be implemented in the client already since we have this information. However, it's not so easy since it would ideally support a time range, which could result in multiple graphs. Maybe that's something for the assets API tooling.

cwasicki · 2025-07-28T11:58:54Z

src/frequenz/client/assets/_client.py

+        with the component.
+    """
+
+    id: int


Needs docstrings.

cwasicki · 2025-07-28T12:00:44Z

src/frequenz/client/assets/_client.py

+
+# pylint: disable=too-many-instance-attributes
+@dataclass
+class ElectricalComponent:


What's the status of these in the common repo? Can't this be used already? https://github.com/frequenz-floss/frequenz-api-common/blob/v0.8.0/proto/frequenz/api/common/v1alpha8/microgrid/electrical_components/electrical_components.proto

cwasicki · 2025-07-28T12:06:33Z

src/frequenz/client/assets/_client.py

+
+
+@dataclass
+class ElectricalComponentConnection:


Same question here, would this one work? https://github.com/frequenz-floss/frequenz-api-common/blob/2678ceef8a1cc0a30cd0f7847d4b84e1462c403b/proto/frequenz/api/common/v1alpha8/microgrid/electrical_components/electrical_components.proto#L644

llucax · 2025-08-20T14:31:21Z

This one was replaced by #21, right @eduardiazf ?

cwasicki · 2025-08-20T16:41:07Z

This one was replaced by #21, right @eduardiazf ?

Only partly, e.g. the connection part is not part of #21.

sandovalrr · 2025-08-21T08:00:24Z

@llucax @cwasicki , Just close this one since @eduardiazf will cover the missing methods in separate PRs

github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Feb 25, 2025

flora-hofmann-frequenz force-pushed the initial_client branch 2 times, most recently from 9d2d8f1 to 818c66e Compare February 27, 2025 14:00

flora-hofmann-frequenz force-pushed the initial_client branch from 818c66e to 21a75dd Compare March 6, 2025 12:51

flora-hofmann-frequenz force-pushed the initial_client branch from 21a75dd to 8dcf561 Compare April 30, 2025 13:45

cwasicki reviewed May 2, 2025

View reviewed changes

flora-hofmann-frequenz force-pushed the initial_client branch from 8dcf561 to 18f523a Compare May 6, 2025 14:04

flora-hofmann-frequenz marked this pull request as ready for review May 6, 2025 14:06

flora-hofmann-frequenz requested a review from a team as a code owner May 6, 2025 14:06

sandovalrr reviewed May 6, 2025

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

flora-hofmann-frequenz force-pushed the initial_client branch 3 times, most recently from 7bd17bd to deb6533 Compare June 9, 2025 09:08

flora-hofmann-frequenz force-pushed the initial_client branch 2 times, most recently from 0797dc2 to 2609589 Compare July 3, 2025 12:40

flora-hofmann-frequenz force-pushed the initial_client branch from 2609589 to e33fa52 Compare July 15, 2025 12:42

Initial client

42b012e

Signed-off-by: Flora <[email protected]>

flora-hofmann-frequenz force-pushed the initial_client branch from e33fa52 to 42b012e Compare July 15, 2025 13:39

flora-hofmann-frequenz requested review from cwasicki and cyiallou July 15, 2025 13:45

cyiallou reviewed Jul 16, 2025

View reviewed changes

cwasicki reviewed Jul 28, 2025

View reviewed changes

sandovalrr closed this Aug 21, 2025



		@dataclass
		class ElectricalComponentConnection:

Initial client #7

Initial client #7

Uh oh!

Conversation

flora-hofmann-frequenz commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flora-hofmann-frequenz commented Jul 15, 2025

Uh oh!

cyiallou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llucax commented Aug 20, 2025

Uh oh!

cwasicki commented Aug 20, 2025

Uh oh!

sandovalrr commented Aug 21, 2025

Uh oh!

Uh oh!

flora-hofmann-frequenz commented Feb 25, 2025 •

edited

Loading